RFR: 8247732: validate user-input intrinsic_ids in ControlIntrinsic [v11]

[Tobias Hartmann]

On Tue, 8 Dec 2020 06:47:32 GMT, Xin Liu <xliu at openjdk.org> wrote:

>> 8247732: validate user-input intrinsic_ids in ControlIntrinsic
>
> Xin Liu has updated the pull request incrementally with one additional commit since the last revision:
> 
>   change to the new CompileCommand format ControlIntrinsic,<method patter>,arguments

Changes requested by thartmann (Reviewer).

src/hotspot/share/compiler/compilerDirectives.hpp line 199:

> 197:         const size_t len = MIN2<size_t>(strlen(*iter), 63) + 1;  // cap len to a value we know is enough for all intrinsic names
> 198:         _bad = NEW_C_HEAP_ARRAY(char, len, mtCompiler);
> 199:         // strncpy always write len characters.if the source string is shorter, the function fills the remaining bytes with NULs.

Some typos, should be: `// strncpy always writes len characters. If the source string is shorter, the function fills the remaining bytes with NULLs.`

src/hotspot/share/compiler/compilerDirectives.hpp line 194:

> 192: 
> 193:  public:
> 194:   ControlIntrinsicValidator(ccstrlist option, bool disabled_all) : _valid(true), _bad(nullptr) {

The argument of `ControlIntrinsicIter` is called `disable_all`, so it should be `disable_all` here as well.

The comment in `ControlIntrinsicIter` is confusing:
// if disable_all is true, it accepts DisableIntrinsic(deprecated) and all intrinsics
// appear in the list are to disable

Could you explain what exactly `disabled_all` is needed for?

src/hotspot/share/compiler/directivesParser.cpp line 331:

> 329:           }
> 330:         }
> 331:         else if (strncmp(option_key->name, "DisableIntrinsic", 16) == 0) {

Please change to `} else if (...) {`.

-------------

PR: https://git.openjdk.java.net/jdk/pull/1179