RFR(S): 8247732: validate user-input intrinsic_ids in ControlIntrinsic
Nils Eliasson
nils.eliasson at oracle.com
Thu Aug 13 15:59:05 UTC 2020
Hi again,
On second thought - please add some basic testing (reuse any old test,
or write a new) that covers the different cases.
I think this table covers all combinations. There should exist tests for
most of them that you can piggy back on.
|+-------------------------------------------------+-------+----------------------------------+
| ControlIntrinsics | valid | invalid |
+-------------------------------------------------+-------+----------------------------------+
| vmflag | ok | print error and don't start |
+-------------------------------------------------+-------+----------------------------------+
| CompilerOracle: -XX:CompileCommand= | ok | print error and continue |
+-------------------------------------------------+-------+----------------------------------+
| CompilerDirectives: -XX:CompilerDirectivesFile= | ok | print error and
don't start |
+-------------------------------------------------+-------+----------------------------------+
| CompilerDirectives via jcmd | ok | print error, vm continues to run |
+-------------------------------------------------+-------+----------------------------------+|
Regards,
Nils
On 2020-08-12 10:21, Nils Eliasson wrote:
> Hi,
>
> Sorry for the delay.
>
> About the error handling:
>
> For CompilerDirectivesFile there are two scenarios:
> 1) If a file containing bad contents is passed on the commandline -
> the VM prints an descriptive error and refuses to start.
> 2) If a file containing bad contents is passed through jcmd - the VM
> prints and error on the jcmd stream and continues to run (ignoring the
> command).
>
> This is achieved by letting the parser just register any parsing
> error, and defer to the caller to decide how to handle the situation.
>
> Regards,
> Nils Eliasson
>
>
> On 2020-08-11 19:09, Liu, Xin wrote:
>> Hi, Reviewers,
>>
>> May I gently ping this?
>>
>> I stuck because I don't know which error handling is appropriate.
>>
>> If we do nothing, current hotspot ignores wrong intrinsic Ids in the
>> cmdline.
>> This patch aborts hotspot when it detects any invalid intrinsic id.
>>
>> thanks,
>> --lx
>>
>>
>> ________________________________________
>> From: Liu, Xin
>> Sent: Monday, August 3, 2020 11:39 PM
>> To: Tobias Hartmann; Nils Eliasson;
>> hotspot-compiler-dev at openjdk.java.net; hotspot-runtime-dev
>> Subject: Re: [EXTERNAL] RFR(S): 8247732: validate user-input
>> intrinsic_ids in ControlIntrinsic
>>
>> hi, Nils,
>>
>> Tobias would like to keep the parser behavior consistency. I think
>> it means that the hotspot need to suppress the warning if the
>> intrinsic_id doesn't exists in compiler directive.
>> eg. -XX:CompileCommand=option,<pattern>,ControlIntrinsic=-_nonexist.
>>
>> What do you think about it?
>>
>> Here is the latest webrev:
>> http://cr.openjdk.java.net/~xliu/8247732/01/webrev/
>>
>> thanks,
>> --lx
>>
>> ________________________________________
>> From: Tobias Hartmann <tobias.hartmann at oracle.com>
>> Sent: Friday, July 24, 2020 2:52 AM
>> To: Liu, Xin; Nils Eliasson; hotspot-compiler-dev at openjdk.java.net;
>> hotspot-runtime-dev
>> Subject: RE: [EXTERNAL] RFR(S): 8247732: validate user-input
>> intrinsic_ids in ControlIntrinsic
>>
>> CAUTION: This email originated from outside of the organization. Do
>> not click links or open attachments unless you can confirm the sender
>> and know the content is safe.
>>
>>
>>
>> Hi Liu,
>>
>> On 23.07.20 18:02, Liu, Xin wrote:
>>> That is my intention too, but CompilerOracle doesn't exit JVM when
>>> it encounters parsing errors.
>>> It just exacts information from CompileCommand as many as possible.
>>> That makes sense because compiler "directives" are supposed to be
>>> optional for program execution.
>>>
>>> I do put the error message in parser's errorbuf. I set a flag
>>> "exit_on_error" to quit JVM after it dumps parser errors. yes, I
>>> treat undefined intrinsics as fatal errors.
>>> This behavior is from Nils comment: "I want to see an error on
>>> startup if the user has specified unknown intrinsic names." It is
>>> also consistent with JVM option -XX:ControlIntrinsic=.
>> Okay, thanks for the explanation! I would prefer consistency in error
>> handling of compiler
>> directives, i.e., handle all parser failures the same way. But I
>> leave it to Nils to decide.
>>
>> Best regards,
>> Tobias
>
More information about the hotspot-compiler-dev
mailing list