RFR [XS]: 8234968: check calloc rv in libinstrument InvocationAdapter

[serguei.spitsyn at oracle.com]

Hi Matthias,

+1

Thanks,
Serguei

On 12/12/19 2:00 AM, Langer, Christoph wrote:
>
> Hi Matthias,
>
> I think your current patch is good as it is – at least it wouldn’t 
> make things worse, AFAICS.
>
> Further improvements can probably be done under another issue.
>
> Cheers
>
> Christoph
>
> *From:* serviceability-dev 
> <serviceability-dev-bounces at openjdk.java.net> *On Behalf Of *Baesken, 
> Matthias
> *Sent:* Freitag, 29. November 2019 08:18
> *To:* Thomas Stüfe <thomas.stuefe at gmail.com>
> *Cc:* serviceability-dev at openjdk.java.net
> *Subject:* [CAUTION] RE: RFR [XS]: 8234968: check calloc rv in 
> libinstrument InvocationAdapter
>
> Hi Thomas, Christoph, thanks for the comments .  Of course the init 
> of  * decodedLen  must be added .
>
> In  case of  returning  NULL  from  decodePath   ,   we would have  
> tmp == NULL  (in char* tmp = func;  )     , assign  tmp to res  and  
> then  we  jplis_assert   , see :
>
> #define TRANSFORM(res,func) {    \
>
>     char* tmp = func;            \
>
>     if (tmp != res) {            \
>
>         free(res);               \
>
>         res = tmp;               \
>
>     }                            \
>
>     jplis_assert((void*)res != (void*)NULL);     \
>
> }
>
> ….
>
> TRANSFORM(path, decodePath(path,&len));
>
> New webrev :
>
> http://cr.openjdk.java.net/~mbaesken/webrevs/8234968.2/
>
> Best regards, Matthias
>
> *From:* Thomas Stüfe <thomas.stuefe at gmail.com 
> <mailto:thomas.stuefe at gmail.com>>
> *Sent:* Freitag, 29. November 2019 07:30
> *To:* Baesken, Matthias <matthias.baesken at sap.com 
> <mailto:matthias.baesken at sap.com>>
> *Cc:* serviceability-dev at openjdk.java.net 
> <mailto:serviceability-dev at openjdk.java.net>
> *Subject:* Re: RFR [XS]: 8234968: check calloc rv in libinstrument 
> InvocationAdapter
>
> Hi Matthias,
>
> I am not certain the callers are prepared to handle NULL.
>
> This is used in a chain of TRANSFORM macro calls which AFAICS do not 
> handle NULL; e.g. , at 872, we pass the returned pointer to 
> convertUft8ToPlatformString which passes it on (on Windows) to 
> MultiByteToWideChar, which does not handle NULL input.
>
> So I wonder whether a clear error message with an exit would be better 
> in this case. Otherwise we may get a crash just some instructions later.
>
> Cheers, Thomas
>
> On Thu, Nov 28, 2019 at 5:21 PM Baesken, Matthias 
> <matthias.baesken at sap.com <mailto:matthias.baesken at sap.com>> wrote:
>
>     Hello, please review this small  patch .
>
>     It adds return value checking for calloc at one place where it is
>     missing .
>
>     Thanks, Matthias
>
>     Bug/webrev :
>
>     https://bugs.openjdk.java.net/browse/JDK-8234968
>
>     http://cr.openjdk.java.net/~mbaesken/webrevs/8234968.1/
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20191213/1cf10ef9/attachment.htm>