Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(241)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 22527004: Drop section rewriting and format fixes. (Closed)

Created:
7 years, 4 months ago by bradn
Modified:
7 years, 4 months ago
Reviewers:
Mark Seaborn, khim
CC:
native-client-reviews_googlegroups.com, Mark Seaborn
Base URL:
http://git.chromium.org/native_client/nacl-glibc.git@master
Visibility:
Public.

Description

Drop section rewriting and format fixes. Turning off section rewriting in fixup_ldso that's not required to satisfy sel_ldr anymore. Formatting fixes left over from: https://codereview.chromium.org/21636003/ BUG=https://code.google.com/p/nativeclient/issues/detail?id=3607 TEST=trybots R=mseaborn@chromium.org Committed: https://git.chromium.org/gitweb?p=native_client/nacl-glibc.git;a=commit;h=5e65ba0

Patch Set 1 #

Total comments: 2

Patch Set 2 : fixes #

Unified diffs Side-by-side diffs Delta from patch set Stats (+19 lines, -20 lines) Patch
M elf/nacl_fixup_ldso.py View 1 chunk +0 lines, -13 lines 0 comments Download
M nacl/dyn-link/ldscripts/elf64_nacl.x View 1 1 chunk +3 lines, -1 line 0 comments Download
M nacl/dyn-link/ldscripts/elf64_nacl.xs View 1 1 chunk +3 lines, -1 line 0 comments Download
M nacl/dyn-link/ldscripts/elf64_nacl.x.static View 1 1 chunk +4 lines, -2 lines 0 comments Download
M nacl/dyn-link/ldscripts/elf_nacl.x View 1 1 chunk +3 lines, -1 line 0 comments Download
M nacl/dyn-link/ldscripts/elf_nacl.xs View 1 1 chunk +3 lines, -1 line 0 comments Download
M nacl/dyn-link/ldscripts/elf_nacl.x.static View 1 1 chunk +3 lines, -1 line 0 comments Download

Messages

Total messages: 6 (0 generated)
bradn
7 years, 4 months ago (2013-08-07 18:30:14 UTC) #1
Mark Seaborn
LGTM with typo fixes. Thanks. https://codereview.chromium.org/22527004/diff/1/nacl/dyn-link/ldscripts/elf64_nacl.x File nacl/dyn-link/ldscripts/elf64_nacl.x (right): https://codereview.chromium.org/22527004/diff/1/nacl/dyn-link/ldscripts/elf64_nacl.x#newcode57 nacl/dyn-link/ldscripts/elf64_nacl.x:57: /* :seg_rodata is on ...
7 years, 4 months ago (2013-08-07 18:34:34 UTC) #2
bradn
https://codereview.chromium.org/22527004/diff/1/nacl/dyn-link/ldscripts/elf64_nacl.x File nacl/dyn-link/ldscripts/elf64_nacl.x (right): https://codereview.chromium.org/22527004/diff/1/nacl/dyn-link/ldscripts/elf64_nacl.x#newcode57 nacl/dyn-link/ldscripts/elf64_nacl.x:57: /* :seg_rodata is on .dummy rather than on .hash ...
7 years, 4 months ago (2013-08-07 19:43:08 UTC) #3
bradn
Committed patchset #2 manually as r5e65ba0 (presubmit successful).
7 years, 4 months ago (2013-08-07 19:44:06 UTC) #4
khim
On Wed, Aug 7, 2013 at 10:30 PM, <bradnelson@google.com> wrote: > Reviewers: Mark Seaborn, > ...
7 years, 4 months ago (2013-08-09 09:31:05 UTC) #5
bradn
7 years, 4 months ago (2013-08-09 16:42:53 UTC) #6 
Sorry forgot to re-run toolchain tryjobs after Mark's review suggestion.
Looking into fixing the loader. Will roll back to the prior revs if that
proves difficult.



On Fri, Aug 9, 2013 at 2:31 AM, Victor Khimenko <khim@chromium.org> wrote:

>
> On Wed, Aug 7, 2013 at 10:30 PM, <bradnelson@google.com> wrote:
>
>> Reviewers: Mark Seaborn,
>>
>> Description:
>> Drop section rewriting and format fixes.
>>
>> Turning off section rewriting in fixup_ldso that's not required to
>> satisfy sel_ldr anymore.
>>
>> If it's not required by sel_ldr anymore then why buildbots are red and
> log is filled with
>
> Error while loading
>
"/b/build/slave/lucid64-glibc/build/native_client/tools/toolchain/linux_x86/x86_64-nacl/lib/runnable-ld.so":
> ELF executable contains an unexpected/unallowed segment/flags
>
> messages?
>
>
>> Formatting fixes left over from:
>>
https://codereview.chromium.**org/21636003/<https://codereview.chromium.org/2...
>>
>>
BUG=https://code.google.com/p/**nativeclient/issues/detail?id=**3607<https://code.google.com/p/nativeclient/issues/detail?id=3607>
>> TEST=trybots
>> R=mseaborn@chromium.org
>>
>> Please review this at
https://codereview.chromium.**org/22527004/<https://codereview.chromium.org/2...
>>
>> SVN Base:
http://git.chromium.org/**native_client/nacl-glibc.git@**master<http://git.ch...
>>
>> Affected files:
>>   M elf/nacl_fixup_ldso.py
>>   M nacl/dyn-link/ldscripts/elf64_**nacl.x
>>   M nacl/dyn-link/ldscripts/elf64_**nacl.x.static
>>   M nacl/dyn-link/ldscripts/elf64_**nacl.xs
>>   M nacl/dyn-link/ldscripts/elf_**nacl.x
>>   M nacl/dyn-link/ldscripts/elf_**nacl.x.static
>>   M nacl/dyn-link/ldscripts/elf_**nacl.xs
>>
>>
>> Index: elf/nacl_fixup_ldso.py
>> diff --git a/elf/nacl_fixup_ldso.py b/elf/nacl_fixup_ldso.py
>> index a5eb1f9eea1e32680b10909f76a2c9**7bb4ab61b1..**
>> 4084d6d458613519dadd2efac742e6**5ffbbaf131 100644
>> --- a/elf/nacl_fixup_ldso.py
>> +++ b/elf/nacl_fixup_ldso.py
>> @@ -43,19 +43,6 @@ def main(args):
>>      check("B", offset_e_type, ET_DYN)
>>      replace("B", offset_e_type, ET_EXEC)
>>
>> -    # sel_ldr rejects ELF Program Headers other than PT_LOAD.
>> -    # Drop PT_DYNAMIC, PT_GNU_STACK and PT_TLS.
>> -    fh.seek(offset_ei_class)
>> -    elfclass = fh.read(1)
>> -    if elfclass == elfclass32:
>> -        check("H", offset_e_phnum32, 7)
>> -        replace("H", offset_e_phnum32, 3)
>> -    elif elfclass == elfclass64:
>> -        check("H", offset_e_phnum64, 7)
>> -        replace("H", offset_e_phnum64, 3)
>> -    else:
>> -        raise AssertionError("Unknown ELF class in file %s." % filename)
>> -
>>      fh.close()
>>
>>
>> Index: nacl/dyn-link/ldscripts/elf64_**nacl.x
>> diff --git a/nacl/dyn-link/ldscripts/**elf64_nacl.x
>> b/nacl/dyn-link/ldscripts/**elf64_nacl.x
>> index 63bb3bc9f84fa7fd31034b1ac1624a**36415760b1..**
>> c3cc5c9c5e398ae1732739203ef36c**f2738907bb 100644
>> --- a/nacl/dyn-link/ldscripts/**elf64_nacl.x
>> +++ b/nacl/dyn-link/ldscripts/**elf64_nacl.x
>> @@ -54,7 +54,9 @@ SECTIONS
>>      *(.note.gnu.build-id)
>>      PROVIDE_HIDDEN (__note_gnu_build_id_end = .);
>>    } :seg_rodata :seg_note
>> -  .dummy : {} : seg_rodata
>> +  /* :seg_rodata is on .dummy rather than on .hash to avoid breaing the
>> linker
>> +     script munging that glibc's Makerules does using sed. */
>> +  .dummy          : {} : seg_rodata
>>    .hash           : { *(.hash) }
>>    .gnu.hash       : { *(.gnu.hash) }
>>    .dynsym         : { *(.dynsym) }
>> Index: nacl/dyn-link/ldscripts/elf64_**nacl.x.static
>> diff --git a/nacl/dyn-link/ldscripts/**elf64_nacl.x.static
>> b/nacl/dyn-link/ldscripts/**elf64_nacl.x.static
>> index 01b261bd35ba8c922b443dc223047f**d0c1b2e8b9..**
>> 3341d985915eccdfacd1f109701535**29c164f539 100644
>> --- a/nacl/dyn-link/ldscripts/**elf64_nacl.x.static
>> +++ b/nacl/dyn-link/ldscripts/**elf64_nacl.x.static
>> @@ -52,8 +52,10 @@ SECTIONS
>>      *(.note.gnu.build-id)
>>      PROVIDE_HIDDEN (__note_gnu_build_id_end = .);
>>    } :seg_rodata :seg_note
>> -  .dummy : {} :seg_rodata
>> -  .hash           : { *(.hash) }
>> +  /* :seg_rodata is on .dummy rather than on .hash to avoid breaing the
>> linker
>> +     script munging that glibc's Makerules does using sed. */
>> +  .dummy          : {} :seg_rodata
>> +  .hash           : { *(.hash) }
>>    .gnu.hash       : { *(.gnu.hash) }
>>    .dynsym         : { *(.dynsym) }
>>    .dynstr         : { *(.dynstr) }
>> Index: nacl/dyn-link/ldscripts/elf64_**nacl.xs
>> diff --git a/nacl/dyn-link/ldscripts/**elf64_nacl.xs
>> b/nacl/dyn-link/ldscripts/**elf64_nacl.xs
>> index 9ef0bf532e96db773734d20b82b924**30ff38784f..**
>> 1c9293328d945b2d78312d5f561367**45befc2987 100644
>> --- a/nacl/dyn-link/ldscripts/**elf64_nacl.xs
>> +++ b/nacl/dyn-link/ldscripts/**elf64_nacl.xs
>> @@ -54,7 +54,9 @@ SECTIONS
>>      *(.note.gnu.build-id)
>>      PROVIDE_HIDDEN (__note_gnu_build_id_end = .);
>>    } :seg_rodata :seg_note
>> -  .dummy : {} : seg_rodata
>> +  /* :seg_rodata is on .dummy rather than on .hash to avoid breaing the
>> linker
>> +     script munging that glibc's Makerules does using sed. */
>> +  .dummy          : {} :seg_rodata
>>    .hash           : { *(.hash) }
>>    .gnu.hash       : { *(.gnu.hash) }
>>    .dynsym         : { *(.dynsym) }
>> Index: nacl/dyn-link/ldscripts/elf_**nacl.x
>> diff --git a/nacl/dyn-link/ldscripts/elf_**nacl.x
>> b/nacl/dyn-link/ldscripts/elf_**nacl.x
>> index 3fabd71fe582faec62e9aa632550c4**6d26a910e5..**
>> 39c77f5ac18fd0bcf5740b956d0b32**18d638a960 100644
>> --- a/nacl/dyn-link/ldscripts/elf_**nacl.x
>> +++ b/nacl/dyn-link/ldscripts/elf_**nacl.x
>> @@ -55,7 +55,9 @@ SECTIONS
>>      *(.note.gnu.build-id)
>>      PROVIDE_HIDDEN (__note_gnu_build_id_end = .);
>>    } :seg_rodata :seg_note
>> -  .dummy : {} :seg_rodata
>> +  /* :seg_rodata is on .dummy rather than on .hash to avoid breaing the
>> linker
>> +     script munging that glibc's Makerules does using sed. */
>> +  .dummy          : {} :seg_rodata
>>    .hash           : { *(.hash) }
>>    .gnu.hash       : { *(.gnu.hash) }
>>    .dynsym         : { *(.dynsym) }
>> Index: nacl/dyn-link/ldscripts/elf_**nacl.x.static
>> diff --git a/nacl/dyn-link/ldscripts/elf_**nacl.x.static
>> b/nacl/dyn-link/ldscripts/elf_**nacl.x.static
>> index 9952daa85564d6d2c53ec8cf477ad9**7375fcb882..**
>> b2da13b107454eb10f709401a45352**6815fc937c 100644
>> --- a/nacl/dyn-link/ldscripts/elf_**nacl.x.static
>> +++ b/nacl/dyn-link/ldscripts/elf_**nacl.x.static
>> @@ -53,7 +53,9 @@ SECTIONS
>>      *(.note.gnu.build-id)
>>      PROVIDE_HIDDEN (__note_gnu_build_id_end = .);
>>    } :seg_rodata :seg_note
>> -  .dummy : {} :seg_rodata
>> +  /* :seg_rodata is on .dummy rather than on .hash to avoid breaing the
>> linker
>> +     script munging that glibc's Makerules does using sed. */
>> +  .dummy          : {} :seg_rodata
>>    .hash           : { *(.hash) }
>>    .gnu.hash       : { *(.gnu.hash) }
>>    .dynsym         : { *(.dynsym) }
>> Index: nacl/dyn-link/ldscripts/elf_**nacl.xs
>> diff --git a/nacl/dyn-link/ldscripts/elf_**nacl.xs
>> b/nacl/dyn-link/ldscripts/elf_**nacl.xs
>> index 57fd8bb1af2262768caa1c01e25ea8**018a8d55c9..**
>> 693509ae2a5569fcb4bf829cdb0c4f**bf7d16b97f 100644
>> --- a/nacl/dyn-link/ldscripts/elf_**nacl.xs
>> +++ b/nacl/dyn-link/ldscripts/elf_**nacl.xs
>> @@ -55,7 +55,9 @@ SECTIONS
>>      *(.note.gnu.build-id)
>>      PROVIDE_HIDDEN (__note_gnu_build_id_end = .);
>>    } :seg_rodata :seg_note
>> -  .dummy : {} :seg_rodata
>> +  /* :seg_rodata is on .dummy rather than on .hash to avoid breaing the
>> linker
>> +     script munging that glibc's Makerules does using sed. */
>> +  .dummy          : {} :seg_rodata
>>    .hash           : { *(.hash) }
>>    .gnu.hash       : { *(.gnu.hash) }
>>    .dynsym         : { *(.dynsym) }
>>
>>
>>
>> --
>> You received this message because you are subscribed to the Google Groups
>> "Native-Client-Reviews" group.
>> To unsubscribe from this group and stop receiving emails from it, send an
>> email to
native-client-reviews+**unsubscribe@googlegroups.com<native-client-reviews%2B...
>> .
>> To post to this group, send email to native-client-reviews@**
>> googlegroups.com <native-client-reviews@googlegroups.com>.
>> Visit this group at http://groups.google.com/**
>>
group/native-client-reviews<http://groups.google.com/group/native-client-reviews>
>> .
>> For more options, visit
https://groups.google.com/**groups/opt_out<https://groups.google.com/groups/o...
>> .
>>
>>
>>
>  --
> You received this message because you are subscribed to the Google Groups
> "Native-Client-Reviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to native-client-reviews+unsubscribe@googlegroups.com.
> To post to this group, send email to
> native-client-reviews@googlegroups.com.
> Visit this group at http://groups.google.com/group/native-client-reviews.
> For more options, visit https://groups.google.com/groups/opt_out.
>
>
>

-- 
You received this message because you are subscribed to the Google Groups
"Native-Client-Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to native-client-reviews+unsubscribe@googlegroups.com.
To post to this group, send email to native-client-reviews@googlegroups.com.
Visit this group at http://groups.google.com/group/native-client-reviews.
For more options, visit https://groups.google.com/groups/opt_out.

Powered by Google App Engine
This is Rietveld 408576698