TMultipartFormData issue with 12.3, but not 11.3

[alank2 5]

This is C++ Builder, but it is using System.Net.Mime.pas which contains TMultipartFormData.

 

Building with 11.3 works fine no errors, but building with 12.3 will cause a crash when I try to delete a pointer to TMultipartFormData.

 

I added System.Net.Mime.pas and stepped into it and it calls:

 

 

If I step over "FStream.Free" it crashes.  If I step into it:

 

 

If I step over Destroy it crashes.  If I step into it, it goes into assembly:

 

 

I can see a lot of differences between the the 22.0 and 23.0 versions of System.Net.Mime.pas.

 

Any ideas on how to I can resolve this with 12.3?

[Brian Evans 117]

Look at the crash details to get hints as to what is going on. Note you didn't post any and "crash" is very little to go on.

 

Guessing it is the CALL that crashes due to the object reference being no longer valid. Perhaps owned objects are getting freed automatically or earlier than before. Not sure which code is yours (if any) so hard to tell.

 

[Remy Lebeau 1568]

@alank2 What you describe implies that either 1) TMultipartFormData is trying to free an invalid FStream object, or 2) you are trying to free an invalid TMultipartFormData object. Unfortunately, there is not enough detail provided to help you. You will have to debug the code for yourself, and double check the user of the pointers involved.

Edited April 5 by Remy Lebeau

[alank2 5]

If I step with F7 into everything it eventually comes to this point:

 

 

 

If I remark out the one line that uses TMultipartFormData, then the issue does not happen:

mfd->AddStream(name, send, value);

 

[alank2 5]

It looks like there was a change to the AddStream - 11.3 left, 12.3 right:

[alank2 5]

If I call it with AOwnsStream=false, it does not crash.

[alank2 5]

I noticed that System.Net.Mime.pas has a deprecated property/message on it:

 

    procedure AddStream(const AField: string; AStream: TStream;
      const AFileName: string = ''; const AContentType: string = '';
      AHeaders: TStrings = nil); overload; deprecated 'Use AddStream with AOwnsStream parameter';

 

In Delphi, does this show up as a warning if you use this procedure?  I didn't get a warning in C++Builder for using it.

Edited April 8 by alank2

[Remy Lebeau 1568]

1 hour ago, alank2 said:

In Delphi, does this show up as a warning if you use this procedure?  I didn't get a warning in C++Builder for using it.

In <System.Net.Mime .hpp>, is there a 'deprecated' attribute on the declaration of AddStream()?  Also, which C++ compiler(s) are you using, exactly?  The DocWiki says only the classic bcc32 compiler supports deprecated messages.

Edited April 8 by Remy Lebeau

[alank2 5]

>In <System.Net.Mime .hpp>, is there a 'deprecated' attribute on the declaration of AddStream()?

 

Yes:

 

    void __fastcall AddStream _DEPRECATED_ATTRIBUTE1("Use AddStream with AOwnsStream parameter") (const System::UnicodeString AField, System::Classes::TStream* AStream, const System::UnicodeString AFileName = System::UnicodeString(), const System::UnicodeString AContentType = System::UnicodeString(), System::Classes::TStrings* AHeaders = (System::Classes::TStrings*)(0x0))/* overload */;

 

I am using the classic compiler:

 

 

I found this:

https://docwiki.embarcadero.com/RADStudio/Sydney/en/DEPRECATED_ATTRIBUTE0_and_DEPRECATED_ATTRIBUTE1

 

If I create a test function and called it:

 

void textzzz()
[[deprecated("use myFunc(int,double) instead")]]
//_DEPRECATED_ATTRIBUTE1("use myFunc(int,double) instead")

{
  ;
}

 

With this I get a warning:  (But the remarked form above doesn't show the "use myFunc(int, double) instead" for some reason.)

 

The big issue is why my call to the deprecated form of AddStream did not give a warning.

 

[Remy Lebeau 1568]

18 minutes ago, alank2 said:

void __fastcall AddStream _DEPRECATED_ATTRIBUTE1("Use AddStream with AOwnsStream parameter") (const System::UnicodeString AField, System::Classes::TStream* AStream, const System::UnicodeString AFileName = System::UnicodeString(), const System::UnicodeString AContentType = System::UnicodeString(), System::Classes::TStrings* AHeaders = (System::Classes::TStrings*)(0x0))/* overload */;

 

I don't know why Embarcadero insists on putting the _DEPRECATED_ATTRIBUTE1 macro in between a function's name and its arguments. That just annoys me.  All examples of [[deprecated]] and __attribute__(deprecated) that I have seen use it either in front or in back of the function declaration, not in the middle.  Even the DocWiki documents it as belonging at the end of a declaration: https://docwiki.embarcadero.com/RADStudio/en/Deprecated

18 minutes ago, alank2 said:

the remarked form above doesn't show the "use myFunc(int, double) instead" for some reason

That is because the _DEPRECATED_ATTRIBUTE1 (and __DEPRECATED_ATTRIBUTE3) macro ignores the message text when you are compiling with the classic compiler (at least in 12.2 - I don't have 12.3 installed yet to check this):

// CLANG vs. BCC attributes
#if !defined(__clang__)
...
#define _DEPRECATED_ATTRIBUTE0        [[deprecated]]
#define _DEPRECATED_ATTRIBUTE1(msg)   [[deprecated]]  // Ignore message as bcc's implementation is capped:(
#define _DEPRECATED_ATTRIBUTE2        [[deprecated]]
#define _DEPRECATED_ATTRIBUTE3(msg)   [[deprecated]]  // Ignore message as bcc's implementation is capped:(
...
#else
...
#define _DEPRECATED_ATTRIBUTE0        // Could use __attribute__((deprecated))
#define _DEPRECATED_ATTRIBUTE1(msg)   // Could use __attribute__((deprecated( msg )))
#define _DEPRECATED_ATTRIBUTE2        __attribute__((deprecated))
#define _DEPRECATED_ATTRIBUTE3(msg)   __attribute__((deprecated( msg )))
...
#endif

This behavior is clearly wrong/outdated for the classic compiler, given that you demonstrated [[deprecated("message")]] is able to display message text just fine, and that behavior is even documented on the DocWiki: https://docwiki.embarcadero.com/RADStudio/en/Deprecated

I have reported the issue: RSS-3285: _DEPRECATED_ATTRIBUTE1 and _DEPRECATED_ATTRIBUTE3 ignore message text for classic compiler

18 minutes ago, alank2 said:

The big issue is why my call to the deprecated form of AddStream did not give a warning.

Given the above macro declarations, the only possibility I can think of is that your real project is not actually compiling with the classic compiler (which you can verify by looking at the build output messages).

[alank2 5]

>I don't know why Embarcadero insists on putting the _DEPRECATED_ATTRIBUTE1 macro in between a function's name and its arguments. That just annoys me.  All 

>examples of [[deprecated]] and __attribute__(deprecated) that I have seen use it either in front or in back of the function declaration, not in the middle.  Even the 

>DocWiki documents it as belonging at the end of a declaration: https://docwiki.embarcadero.com/RADStudio/en/Deprecated

 

I agree.

 

>That is because the _DEPRECATED_ATTRIBUTE1 (and __DEPRECATED_ATTRIBUTE3) macro ignores the message text when you are compiling with the classic compiler (at least in 12.2 - I don't have 12.3 installed yet to check this):

 

At least that is explained!

 

>This behavior is clearly wrong/outdated for the classic compiler, given that you demonstrated [[deprecated("message")]] is able to display message text just 

>fine, and that behavior is even documented on the DocWiki: https://docwiki.embarcadero.com/RADStudio/en/Deprecated

>I have reported the issue: RSS-3285: _DEPRECATED_ATTRIBUTE1 and _DEPRECATED_ATTRIBUTE3 ignore message text for classic compiler

 

Thank you!

 

>Given the above macro declarations, the only possibility I can think of is that your real project is not actually compiling with the classic compiler (which you can verify by looking at the build output messages).

 

I turn on all warnings except 8057 parameter is never used - bcc32 is the classic compiler, right?

 

bcc32 command line for "webapi.cpp"
  c:\program files (x86)\embarcadero\studio\23.0\bin\bcc32.exe -DNDEBUG;QDX_MODE=1;;FRAMEWORK_VCL -n.\Win32\Release -I"c:\program files
  (x86)\embarcadero\studio\23.0\include\windows\vcl";"c:\program files (x86)\embarcadero\studio\23.0\source\rtl\net";"c:\program files
  (x86)\embarcadero\studio\23.0\include";"c:\program files (x86)\embarcadero\studio\23.0\include\dinkumware";"c:\program files
  (x86)\embarcadero\studio\23.0\include\windows\crtl";"c:\program files (x86)\embarcadero\studio\23.0\include\windows\sdk";"c:\program files
  (x86)\embarcadero\studio\23.0\include\windows\rtl";"c:\program files (x86)\embarcadero\studio\23.0\include\windows\vcl";"c:\program files
  (x86)\embarcadero\studio\23.0\include\windows\fmx";C:\Users\Public\Documents\Embarcadero\Studio\23.0\hpp\Win32;"C:\Program Files (x86)\Devart\SDAC
  for RAD Studio 12\Include\Win32";C:\Users\Public\Documents\Embarcadero\Studio\23.0\hpp\Win32 -Q -c -tM -tW -C8 -o.\Win32\Release\webapi.obj -wamb
  -wamp -wasm -wbbf -wcln -wdef -winl -wnak -wnod -w-par -wpin -wsig -wstu -wstv -wucp -wuse -wprc -wstl -wexc -wimp -wntn -wpad -wiac -wbcx -wpun -O2
  -v- -vi -H=.\Win32\Release\s4ebet.pch -H webapi.cpp

[Remy Lebeau 1568]

Got a reply on my ticket:

Quote

From what I recall, ignoring the message is intentional. I’ll confirm the reason but I seem to recall that it’s because the classic compiler has a hardcoded limit on how many deprecated w/ message it can support. IOW, there’s a limit on the number supported… past that number, the compiler ignores the remaining.

1 hour ago, alank2 said:

bcc32 is the classic compiler, right?

Yes

 

[alank2 5]

I started a brand new windows vcl project, switched to 32 bit, set the classic compiler and added these lines of code to see if I would get a warning:

 

  TMultipartFormData *mfd;
  TMemoryStream *send;

  send=new TMemoryStream();
  mfd=new TMultipartFormData(true);
  mfd->AddStream("name", send, "value");

 

No warning.  What annoys me here is that if I had had the warning it would have tipped me off to the problem much sooner.

[Remy Lebeau 1568]

18 hours ago, alank2 said:

added these lines of code to see if I would get a warning:

...

No warning.

Got an interesting reply on the ticket:

Quote

I will confirm this but I don’t believe the above should generate the warning because I believe the compiler will invoke the overload of .AddStream that takes a Boolean as 3rd parameter. This has been a classic problem when Delphi refacators APIs because a constant string literal will convert to bool in C++ (but not in Delphi).

IOW, I believe the code shown above…

Is Invoking	procedure AddStream(const AField: string; AStream: TStream; AOwnsStream: Boolean; const AFileName: string = ''; const AContentType: string = ''; AHeaders: TStrings = nil); overload;
instead of the deprecated	procedure AddStream(const AField: string; AStream: TStream; AOwnsStream: Boolean; const AFileName: string = ''; const AContentType: string = ''; AHeaders: TStrings = nil); overload;

 

You would have to cast the third string literal to UnicdeString to ensure it’s invoking the deprecated overload.

He meant this deprecated method instead:

procedure AddStream(const AField: string; AStream: TStream;
const AFileName: string = ''; const AContentType: string = '';
AHeaders: TStrings = nil); overload; deprecated 'Use AddStream with AOwnsStream parameter';

But yeah, that makes total sense now, C++'s overload resolution would indeed prefer to implicitly convert a string literal into a bool rather than a user-defined object (ie UnicodeString), and that would explain your crash if AOwnsStream were implicitly being set to 'true' and then you try to free both streams yourself.

 

So, do the conversion explicitly and then you will hopefully see the warning:

mfd->AddStream(_D("name"), send, String(_D("value")));

 

Edited Thursday at 06:43 PM by Remy Lebeau

[alank2 5]

>But yeah, that makes total sense now, C++'s overload resolution would indeed prefer to implicitly convert a string literal into a bool rather than a user-defined 

>object (ie UnicodeString), and that would explain your crash if AOwnsStream were implicitly being set to 'true' and then you try to free both streams yourself.

 

I have seen this type of thing before and wondered if that is what could be going on.  I've run into it before changing a char* pointer to an integer expecting deprecated code to catch it as an error/warning reminding me to correct it, but it doesn't.

 

void myfunc(bool A)
{
  if (A)
    Form1->Caption="true";
  else Form1->Caption="false";
}

 

  myfunc("Hello");    //calls as true
  myfunc(NULL);    //calls as false

 

In this case, it is evaluating the string pointer as non-zero and becoming true. but NULL is false.

 

>So, do the conversion explicitly and then you will hopefully see the warning:

 

Yes - indeed so! :

[Remy Lebeau 1568]

4 hours ago, alank2 said:

I've run into it before changing a char* pointer to an integer expecting deprecated code to catch it as an error/warning reminding me to correct it, but it doesn't.

Pointers are implicitly convertible to bool.  A string literal is not itself a pointer, but it does decay into a pointer, which is then convertible.

Quote

In this case, it is evaluating the string pointer as non-zero and becoming true. but NULL is false.

Note that NULL is not a pointer.  It is an alias for a null pointer constant, either nullptr or integer literal 0 (depending on implementation), which can be assigned to any pointer.

 

Edited Friday at 12:43 AM by Remy Lebeau