Why do I get access violations when a control's class name is very, very long?
Asked Answered
W

1

23

I subclassed a control in order so I can add a few fields that I need, but now when I create it at runtime I get an Access Violation. Unfortunately this Access Violation doesn't happen at the place where I'm creating the control, and even those I'm building with all debug options enabled (including "Build with debug DCU's") the stack trace doesn't help me at all!

In my attempt to reproduce the error I tried creating a console application, but apparently this error only shows up in a Forms application, and only if my control is actually shown on a form!

Here are the steps to reproduce the error. Create a new VCL Forms application, drop a single button, double-click to create the OnClick handler and write this:

type TWinControl<T,K,W> = class(TWinControl);

procedure TForm3.Button1Click(Sender: TObject);
begin
  with TWinControl<TWinControl, TWinControl, TWinControl>.Create(Self) do
  begin
    Parent := Self;
  end;
end;

This successively generates the Access Violation, every time I tried. Only tested this on Delphi 2010 as that's the only version I've got on this computer.

The questions would be:

  • Is this a known bug in Delphi's Generics?
  • Is there a workaround for this?

Edit

Here's the link to the QC report: http://qc.embarcadero.com/wc/qcmain.aspx?d=112101

Wrapped answered 21/1, 2013 at 20:49 Comment(16)
Is it important that the name of the generic class and the type arguments be the same?Linguistic
Not important. I'm writing the answer right now, it'll take a few more minutes.Wrapped
Next time you already know the answer to the question you're posting, click the "answer your own question" box so you can post the question and the answer simultaneously.Linguistic
That's what I'm doing, but it takes time to write the answer. Notice the first line of the question...Wrapped
I mean the checkbox on the "ask a question" page. You can ask and answer simultaneously.Linguistic
That's a nice touch, I'll remember for next time. Whenever that might be.Wrapped
Currently, the title is or reveals the answer.Churchy
@NGLN, feel free to edit and make the title more useful. I have no idea how one might run into this again, but it sure took me a while to figure this out, so I hope it'll help somebody.Wrapped
I was busy debugging this and about to post an answer when yours appeared. I had not realised we were playing a game. Ah well. You can probably intercept TWinControl.CreateParams and fix this from the inside. Have you submitted a QC report yet?Sheffy
@DavidHeffernan, we were not playing a game, look at the first version of the question: I did specify I know the answer and I'm about to write it. Of course, with the current title it shouldn't take a lot of debugging to figure out.Wrapped
Yeah, well I missed that. I focussed on the code. You should have answered immediately. You say that it takes time to write an answer, well you can just write the answer before you post the question.Sheffy
So, are you going to report to QC? Also, the question would be far better if you just boiled it down to a long class name. The generics is totally misleading. Try and make the question as simple as possible to get across the problem.Sheffy
@DavidHeffernan, got a small problem with my QC account, I'll report it tomorrow morning from work, where I've got my password written down. (I use exclusively "named networked" licenses so I never need my EDN account - except for reporting bugs). As for making the question as simple as possible... I don't know, I think a reference to generics in the title makes it more likely to be found. I doubt anyone uses 64 char long class names without generics!Wrapped
@DavidHeffernan: qc.embarcadero.com/wc/qcmain.aspx?d=112101Wrapped
@Cosmin just add the link to the question body pleaseMincing
Note that QualityCentral has now been shut down, so you can't access qc.embarcadero.com links anymore. If you need access to old QC data, look at QCScraper.Confrere
W
27

First of all, this has nothing to do with generics, but is a lot more likely to manifest when generics are being used. It turns out there's a buffer overflow bug in TControl.CreateParams. If you look at the code, you'll notice it fills a TCreateParams structure, and especially important, it fills the TCreateParams.WinClassName with the name of the current class (the ClassName). Unfortunately WinClassName is a fixed length buffer of only 64 char's, but that needs to include the NULL-terminator; so effectively a 64 char long ClassName will overflow that buffer!

It can be tested with this code:

TLongWinControlClassName4567890123456789012345678901234567891234 = class(TWinControl)
end;

procedure TForm3.Button1Click(Sender: TObject);
begin
  with TLongWinControlClassName4567890123456789012345678901234567891234.Create(Self) do
  begin
    Parent := Self;
  end;
end;

That class name is exactly 64 characters long. Make it one character shorter and the error goes away!

This is a lot more likely to happen when using generics because of the way Delphi constructs the ClassName: it includes the unit name where the parameter type is declared, plus a dot, then the name of the parameter type. For example, the TWinControl<TWinControl, TWinControl, TWinControl> class has the following ClassName:

TWinControl<Controls.TWinControl,Controls.TWinControl,Controls.TWinControl>

That's 75 characters long, over the 63 limit.

Workaround

I adopted a simple error message from the potentially-error-generating class. Something like this, from the constructor:

constructor TWinControl<T, K, W>.Create(aOwner: TComponent);
begin
  {$IFOPT D+}
  if Length(ClassName) > 63 then raise Exception.Create('The resulting ClassName is too    long: ' + ClassName);
  {$ENDIF}
  inherited;
end;

At least this shows a decent error message that one can immediately act upon.

Later Edit, True Workaround

The previous solution (raising an error) works fine for a non-generic class that has a realy-realy long name; One would very likely be able to shorten it, make it 63 chars or less. That's not the case with generic types: I ran into this problem with a TWinControl descendant that took 2 type parameters, so it was of the form:

TMyControlName<Type1, Type2>

The gnerate ClassName for a concrete type based on this generic type takes the form:

TMyControlName<UnitName1.Type1,UnitName2.Type2>

so it includes 5 identifiers (2x unit identifier + 3x type identifier) + 5 symbols (<.,.>); The average length of those 5 identifiers need to be less then 12 chars each, or else the total length is over 63: 5x12+5 = 65. Using only 11-12 characters per identifier is very little and goes against best practices (ie: use long descriptive names because keystrokes are free!). Again, in my case, I simply couldn't make my identifiers that short.

Considering how shortening the ClassName is not always possible, I figured I'd attempt removing the cause of the problem (the buffer overflow). Unfortunately that's very difficult because the error originates from TWinControl.CreateParams, at the bottom of the CreateParams hierarchy. We can't NOT call inherited because CreateParams is used all along the inheritance chain to build the window creation parameters. Not calling it would require duplicating all the code in the base TWinControl.CreateParams PLUS all the code in intermediary classes; It would also not be very portable, since any of that code might change with future versions of the VCL (or future version of 3rd party controls we might be subclassing).

The following solution doesn't stop TWinControl.CreateParams from overflowing the buffer, but makes it harmless and then (when the inherited call returns) fixes the problem. I'm using a new record (so I have control over the layout) that includes the original TCreateParams but pads it with lots of space for TWinControl.CreateParams to overflow into. TWinControl.CreateParams overflows all it wants, I then read the complete text and make it so it fits the original bounds of the record also making sure the resulting shortened name is reasonably likely to be unique. I'm including the a HASH of the original ClassName in the WndName to help with the uniqueness issue:

type
  TWrappedCreateParamsRecord = record
    Orignial: TCreateParams;
    SpaceForCreateParamsToSafelyOverflow: array[0..2047] of Char;
  end;

procedure TExtraExtraLongWinControlDescendantClassName_0123456789_0123456789_0123456789_0123456789.CreateParams(var Params: TCreateParams);
var Wrapp: TWrappedCreateParamsRecord;
    Hashcode: Integer;
    HashStr: string;
begin
  // Do I need to take special care?
  if Length(ClassName) >= Length(Params.WinClassName) then
    begin
      // Letting the code go through will cause an Access Violation because of the
      // Buffer Overflow in TWinControl.CreateParams; Yet we do need to let the
      // inherited call go through, or else parent classes don't get the chance
      // to manipulate the Params structure. Since we can't fix the root cause (we
      // can't stop TWinControl.CreateParams from overflowing), let's make sure the
      // overflow will be harmless.
      ZeroMemory(@Wrapp, SizeOf(Wrapp));
      Move(Params, Wrapp.Orignial, SizeOf(TCreateParams));
      // Call the original CreateParams; It'll still overflow, but it'll probably be hurmless since we just
      // padded the orginal data structure with a substantial ammount of space.
      inherited CreateParams(Wrapp.Orignial);
      // The data needs to move back into the "Params" structure, but before we can do that
      // we should FIX the overflown buffer. We can't simply trunc it to 64, and we don't want
      // the overhead of keeping track of all the variants of this class we might encounter.
      // Note: Think of GENERIC classes, where you write this code once, but there might
      // be many-many different ClassNames at runtime!
      //
      // My idea is to FIX this by keeping as much of the original name as possible, but
      // including the HASH value of the full name into the window name; If the HASH function
      // is any good then the resulting name as a very high probability of being Unique. We'll
      // use the default Hash function used for Delphi's generics.
      HashCode := TEqualityComparer<string>.Default.GetHashCode(PChar(@Wrapp.Orignial.WinClassName));
      HashStr := IntToHex(HashCode, 8);
      Move(HashStr[1], Wrapp.Orignial.WinClassName[High(Wrapp.Orignial.WinClassName)-8], 8*SizeOf(Char));
      Wrapp.Orignial.WinClassName[High(Wrapp.Orignial.WinClassName)] := #0;
      // Move the TCreateParams record back were we've got it from
      Move(Wrapp.Orignial, Params, SizeOf(TCreateParams));
    end
  else
    inherited;
end;
Wrapped answered 21/1, 2013 at 21:3 Comment(2)
The API allows for 256 chars, I wonder why would anyone come up with a 64 limit..Undermine
My guess is that lpszClassName probably was limited to 64 chars way way back in the early days, but then later got increased to 256 and nobody every fixed the VCL. It was addressed in Delphi.NET and FMX, but is still broken in VCL to this day (at least in 10.0 Seattle, I haven't checked later versions). I have filed a new bug report in Quality Portal: RSP-18399: Buffer overflow in TWinControl.CreateParams()Confrere

© 2022 - 2024 — McMap. All rights reserved.