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

[Langer, Christoph]

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/20191212/8b401c1d/attachment-0001.htm>