[JSC] Make disassembler data structures constant read-only data

[Yusuke Suzuki]

[JSC] Make disassembler data structures constant ro data

[Yusuke Suzuki]

Created attachment 360611 [details]
Patch

[EWS Watchlist]

Attachment 360611 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/disassembler/udis86/ud_itab.py:252:  whitespace after '('  [pep8/E201] [5]
ERROR: Source/JavaScriptCore/disassembler/udis86/ud_itab.py:263:  multiple statements on one line (semicolon)  [pep8/E702] [5]
ERROR: Source/JavaScriptCore/disassembler/udis86/ud_itab.py:263:  whitespace after '('  [pep8/E201] [5]
ERROR: Source/JavaScriptCore/disassembler/udis86/ud_itab.py:296:  whitespace after '('  [pep8/E201] [5]
ERROR: Source/JavaScriptCore/disassembler/udis86/ud_itab.py:326:  whitespace after '('  [pep8/E201] [5]
Total errors found: 5 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Mark Lam]

Comment on attachment 360611 [details]
Patch

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

r=me

> Source/JavaScriptCore/ChangeLog:3
> +        [JSC] Make disassembler data structures constant ro data

can you spell out read-only please?

[Yusuke Suzuki]

Comment on attachment 360611 [details]
Patch

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

>> Source/JavaScriptCore/ChangeLog:3
>> +        [JSC] Make disassembler data structures constant ro data
> 
> can you spell out read-only please?

Fixed.

[Yusuke Suzuki]

Committed r240730: <https://trac.webkit.org/changeset/240730>

[Radar WebKit Bug Importer]

<rdar://problem/47680981>

[Michael Catanzaro]

Committed r240755: <https://trac.webkit.org/changeset/240755>

[Michael Catanzaro]

(In reply to Michael Catanzaro from comment #7)
> Committed r240755: <https://trac.webkit.org/changeset/240755>

The output is kinda ugly:

const struct ud_lookup_table_list_entry ud_lookup_table_list[] = {
    /* 000 */ { ud_itab__0, UD_TAB__OPC_TABLE, (const char* const)"opctbl" },
    /* 001 */ { ud_itab__1, UD_TAB__OPC_MODE, (const char* const)"/m" },
    /* 002 */ { ud_itab__2, UD_TAB__OPC_MODE, (const char* const)"/m" },
    /* 003 */ { ud_itab__3, UD_TAB__OPC_MODE, (const char* const)"/m" },
    /* 004 */ { ud_itab__4, UD_TAB__OPC_TABLE, (const char* const)"opctbl" },

etc. But it's needed to avoid:

In file included from ../../Source/JavaScriptCore/disassembler/udis86/udis86_itab_holder.c:30:
DerivedSources/JavaScriptCore/udis86_itab.c:2677:35: error: conflicting types for ‘ud_lookup_table_list’
 struct ud_lookup_table_list_entry ud_lookup_table_list[] = {
                                   ^~~~~~~~~~~~~~~~~~~~
In file included from DerivedSources/JavaScriptCore/udis86_itab.c:2,
                 from ../../Source/JavaScriptCore/disassembler/udis86/udis86_itab_holder.c:30:
../../Source/JavaScriptCore/disassembler/udis86/udis86_decode.h:189:48: note: previous declaration of ‘ud_lookup_table_list’ was here
 extern const struct ud_lookup_table_list_entry ud_lookup_table_list[];
                                                ^~~~~~~~~~~~~~~~~~~~
In file included from ../../Source/JavaScriptCore/disassembler/udis86/udis86_itab_holder.c:30:
DerivedSources/JavaScriptCore/udis86_itab.c:3272:22: error: conflicting types for ‘ud_itab’
 struct ud_itab_entry ud_itab[] = {
                      ^~~~~~~
In file included from DerivedSources/JavaScriptCore/udis86_itab.c:2,
                 from ../../Source/JavaScriptCore/disassembler/udis86/udis86_itab_holder.c:30:
../../Source/JavaScriptCore/disassembler/udis86/udis86_decode.h:188:35: note: previous declaration of ‘ud_itab’ was here
 extern const struct ud_itab_entry ud_itab[];
                                   ^~~~~~~

[Fujii Hironori]

Same error message was reported for WinCairo.
https://build.webkit.org/builders/WinCairo%2064-bit%20WKL%20Release%20%28Build%29/builds/6570

They are solved by triggering Force Clean Build.
https://build.webkit.org/builders/WinCairo%2064-bit%20WKL%20Release%20%28Build%29/builds/6581

Seems a Ninja incremental build issue.

[Fujii Hironori]

According to the GTK build log, it seems that udis86_itab_holder.c was recompiled without regenerating udis86_itab.c by invoking ud_itab.py.
https://build.webkit.org/builders/GTK%20Linux%2064-bit%20Release%20%28Build%29/builds/18892/steps/compile-webkit/logs/stdio

[Michael Catanzaro]

Hmm, OK I tricked myself then; I manually deleted udis86_itab.c to test that change, just deleting it was probably enough. Indeed, changes to ud_itab.py don't result in a rebuild of the generated files.

I'll rollout my "fix."

[Michael Catanzaro]

Reverted r240755 for reason:

This was not correct

Committed r240760: <https://trac.webkit.org/changeset/240760>

[Michael Catanzaro]

Triggered clean builds on all the WPE and GTK bots. Surprisingly, the JSCOnly bots don't seem to need it for some reason, not sure why.

[Fujii Hironori]

Because we have no JSCOnly x86 bots compiling UDIS86 code path.

It seems difficult to attach a source dependency from ud_itab.py custom commad to udis86_itab_holder.c because it is difficult to determine which unified source bundle will includes udis86_itab_holder.c.
Anyway, I think ud_itab.py won't be modified frequently.

[Fujii Hironori]

I'm wrong. C files are not bundled. File a new ticker Bug 194085 to the incremental Ninja build issue.