Bug 229841

Summary:

[JSC] Fix WebKitAdditions directory traversal in offlineasm

Product:

WebKit

Reporter:

Yusuke Suzuki <ysuzuki>

Component:

New Bugs

Assignee:

Yusuke Suzuki <ysuzuki>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

ews-watchlist, keith_miller, mark.lam, msaboff, saam, tzagallo, webkit-bug-importer

Priority:

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>