Summary: | [JSC] Segfault on ARM64 with gcc in elf_dynamic_do_Rela | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Akos Kiss <akiss> | ||||||
Component: | JavaScriptCore | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | commit-queue, dbates, fpizlo, msaboff, oliver, ossy | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 108645 | ||||||||
Attachments: |
|
Description
Akos Kiss
2014-10-05 07:09:36 PDT
Created attachment 239299 [details]
Proposed patch.
Update: According to conversations with gcc devs (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63461), it seems that this is not a bug in gcc but expected (alternatively: intentionally undefined) behaviour. The solution proposed at gcc bugzilla is to use .pushsection/.popsection, however, I'd still propose for the current .text/.data approach. .text is apropriately translated to .section __TEXT,__text on Mac and to .section .text on Linux (.data similarly), while .pushsection ".text" would not be so portable. If I got it right. (In reply to comment #2) > Update: According to conversations with gcc devs (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63461), it seems that this is not a bug in gcc but expected (alternatively: intentionally undefined) behaviour. > > The solution proposed at gcc bugzilla is to use .pushsection/.popsection, however, I'd still propose for the current .text/.data approach. .text is apropriately translated to .section __TEXT,__text on Mac and to .section .text on Linux (.data similarly), while .pushsection ".text" would not be so portable. If I got it right. What about using .previous instead of .data? That should be more portable. Created attachment 239527 [details]
Proposed patch, v2
You're right, .previous works -- on my ARM64/Linux at least. (On that box, I've been experimenting with some corner cases to check that different compilers -- gcc & clang -- behave as expected, and found no issues, fortunately. I've also tried out some .pushsection/.popsection-based approaches but all turned out to need more and potentially harder to maintain changes.) I could not validate the patch for iOS.
Interestingly, all the other defines use the same .text-based approach for defining the thunks with no .data/.previous/.pushsection-.popsection around and noone ran into this problem before. Still I guess that it would be safer to have .previous in those macros as well, but following the "if it's not broken, don't fix it" line, I've made no changes there.
Comment on attachment 239527 [details]
Proposed patch, v2
r=me
Comment on attachment 239527 [details] Proposed patch, v2 Clearing flags on attachment: 239527 Committed r174503: <http://trac.webkit.org/changeset/174503> All reviewed patches have been landed. Closing bug. |