Bug 229841

Summary: [JSC] Fix WebKitAdditions directory traversal in offlineasm
Product: WebKit Reporter: Yusuke Suzuki <ysuzuki>
Component: New BugsAssignee: Yusuke Suzuki <ysuzuki>
Status: RESOLVED FIXED    
Severity: Normal CC: ews-watchlist, keith_miller, mark.lam, msaboff, sbarati, tzagallo, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch mark.lam: review+

Description Yusuke Suzuki 2021-09-02 16:45:17 PDT
[JSC] Fix WebKitAddition directory traversal in offlineasm
Comment 1 Yusuke Suzuki 2021-09-02 16:47:45 PDT
Created attachment 437219 [details]
Patch
Comment 2 Mark Lam 2021-09-02 17:00:56 PDT
Comment on attachment 437219 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=437219&action=review

r=me

> Source/JavaScriptCore/offlineasm/asm.rb:338
> +    opts.banner = "Usage: asm.rb asmFile offsetsFile outputFileName [--assembler=<ASM> --use-deployment-location]"

Should this be "[--assembler=<ASM>] [--use-deployment-location]" instead since the 2 options are independent?

> Source/JavaScriptCore/offlineasm/generate_offset_extractor.rb:52
> +    # This option is currently only used to specify the masm assembler

This comment is not relevant.  I think it's just copied from above.  Please remove.

> Source/JavaScriptCore/offlineasm/generate_settings_extractor.rb:49
> +    # This option is currently only used to specify the masm assembler

Ditto.  Please remove.
Comment 3 Yusuke Suzuki 2021-09-02 17:16:54 PDT
Comment on attachment 437219 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=437219&action=review

Thanks!

>> Source/JavaScriptCore/offlineasm/asm.rb:338
>> +    opts.banner = "Usage: asm.rb asmFile offsetsFile outputFileName [--assembler=<ASM> --use-deployment-location]"
> 
> Should this be "[--assembler=<ASM>] [--use-deployment-location]" instead since the 2 options are independent?

Changed.

>> Source/JavaScriptCore/offlineasm/generate_offset_extractor.rb:52
>> +    # This option is currently only used to specify the masm assembler
> 
> This comment is not relevant.  I think it's just copied from above.  Please remove.

Removed.

>> Source/JavaScriptCore/offlineasm/generate_settings_extractor.rb:49
>> +    # This option is currently only used to specify the masm assembler
> 
> Ditto.  Please remove.

Removed.
Comment 4 Mark Lam 2021-09-02 17:18:55 PDT
Comment on attachment 437219 [details]
Patch

You missed one spot in parser.rb: search for "/usr/local/include/WebKitAdditions/" and you'll see a seconds place the path is chosen in `def parseIncludes(final, comment)`.
Comment 5 Yusuke Suzuki 2021-09-02 17:32:35 PDT
Created attachment 437225 [details]
Patch
Comment 6 Mark Lam 2021-09-02 17:34:23 PDT
Comment on attachment 437225 [details]
Patch

r=me
Comment 7 Radar WebKit Bug Importer 2021-09-02 18:07:13 PDT
<rdar://problem/82701787>
Comment 8 Yusuke Suzuki 2021-09-02 19:13:45 PDT
OK, compilation finished.
Comment 9 Yusuke Suzuki 2021-09-02 19:14:56 PDT
Committed r281972 (241279@main): <https://commits.webkit.org/241279@main>