[PATCH-Android,android/external/elfcopy] Modified to eliminate build errors using GCC 4.5/4.6.

Message ID 1300252598-1653-4-git-send-email-sachin.kamat@linaro.org
State Accepted
Headers show

Commit Message

Sachin Kamat March 16, 2011, 5:16 a.m.
Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>

Change-Id: I01286388a1bef915cb06adae862d9f3a0397b059
---
 elfcopy.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

Comments

Arnd Bergmann March 16, 2011, 11:47 a.m. | #1
On Wednesday 16 March 2011, Sachin Kamat wrote:
> --- a/elfcopy.c
> +++ b/elfcopy.c
> @@ -2456,7 +2456,6 @@ update_symbol_values(Elf *elf, GElf_Ehdr *ehdr,
>                             out why and also figure out whether the zero value should have
>                             been adjusted, after all.
>                          */
> -                        ASSERT(!(shdr_info[sym->st_shndx].shdr.sh_flags & SHF_ALLOC));
>                          ASSERT(shdr_info[i].shdr.sh_type == SHT_SYMTAB);
>                      }

Like the -Werror changes that Alexander commented on, this change
circumvents the error output, but does not address the underlying
problem. If you remove an assertion, that at least needs a very
good explanation why the assertion was incorrect in the first
place.

	Arnd
Sachin Kamat March 16, 2011, 12:01 p.m. | #2
Hi Arnd, Alexander,

Some of the work arounds are just make shift solutions so that those
dependent on the build can carry on with their work.
Meanwhile as you guys said, the underlying problem needs to be addressed and
fixed. This might take a while.

Regards
Sachin

On 16 March 2011 17:17, Arnd Bergmann <arnd@arndb.de> wrote:

> On Wednesday 16 March 2011, Sachin Kamat wrote:
> > --- a/elfcopy.c
> > +++ b/elfcopy.c
> > @@ -2456,7 +2456,6 @@ update_symbol_values(Elf *elf, GElf_Ehdr *ehdr,
> >                             out why and also figure out whether the zero
> value should have
> >                             been adjusted, after all.
> >                          */
> > -                        ASSERT(!(shdr_info[sym->st_shndx].shdr.sh_flags
> & SHF_ALLOC));
> >                          ASSERT(shdr_info[i].shdr.sh_type == SHT_SYMTAB);
> >                      }
>
> Like the -Werror changes that Alexander commented on, this change
> circumvents the error output, but does not address the underlying
> problem. If you remove an assertion, that at least needs a very
> good explanation why the assertion was incorrect in the first
> place.
>
>        Arnd
>
Alexander Sack March 16, 2011, 12:35 p.m. | #3
Ack, we discussed this today in android meeting and will take your
patches to a special toolchain branch that allows us to experiement
with 4.5/4.6 toolchain while not hiding the underlying problems.
Thanks for your work!

On Wed, Mar 16, 2011 at 1:01 PM, Sachin Kamat <sachin.kamat@linaro.org> wrote:
> Hi Arnd, Alexander,
>
> Some of the work arounds are just make shift solutions so that those
> dependent on the build can carry on with their work.
> Meanwhile as you guys said, the underlying problem needs to be addressed and
> fixed. This might take a while.
>
> Regards
> Sachin
>
> On 16 March 2011 17:17, Arnd Bergmann <arnd@arndb.de> wrote:
>>
>> On Wednesday 16 March 2011, Sachin Kamat wrote:
>> > --- a/elfcopy.c
>> > +++ b/elfcopy.c
>> > @@ -2456,7 +2456,6 @@ update_symbol_values(Elf *elf, GElf_Ehdr *ehdr,
>> >                             out why and also figure out whether the zero
>> > value should have
>> >                             been adjusted, after all.
>> >                          */
>> > -                        ASSERT(!(shdr_info[sym->st_shndx].shdr.sh_flags
>> > & SHF_ALLOC));
>> >                          ASSERT(shdr_info[i].shdr.sh_type ==
>> > SHT_SYMTAB);
>> >                      }
>>
>> Like the -Werror changes that Alexander commented on, this change
>> circumvents the error output, but does not address the underlying
>> problem. If you remove an assertion, that at least needs a very
>> good explanation why the assertion was incorrect in the first
>> place.
>>
>>        Arnd
>
>
>
> --
> With warm regards,
> Sachin
>
Jamie Bennett March 16, 2011, 1:20 p.m. | #4
On 16/03/11 at 05:31pm, Sachin Kamat wrote:
> Hi Arnd, Alexander,
> 
> Some of the work arounds are just make shift solutions so that those
> dependent on the build can carry on with their work.
> Meanwhile as you guys said, the underlying problem needs to be addressed and
> fixed. This might take a while.

Maybe a comment just above each of these 'make shift solutions' will help 
you a) locate them later to fix up and b) give commentary on why you are 
temporarily doing them.
 
> Regards
> Sachin

Regards,
Jamie.
 
> On 16 March 2011 17:17, Arnd Bergmann <arnd@arndb.de> wrote:
> 
> > On Wednesday 16 March 2011, Sachin Kamat wrote:
> > > --- a/elfcopy.c
> > > +++ b/elfcopy.c
> > > @@ -2456,7 +2456,6 @@ update_symbol_values(Elf *elf, GElf_Ehdr *ehdr,
> > >                             out why and also figure out whether the zero
> > value should have
> > >                             been adjusted, after all.
> > >                          */
> > > -                        ASSERT(!(shdr_info[sym->st_shndx].shdr.sh_flags
> > & SHF_ALLOC));
> > >                          ASSERT(shdr_info[i].shdr.sh_type == SHT_SYMTAB);
> > >                      }
> >
> > Like the -Werror changes that Alexander commented on, this change
> > circumvents the error output, but does not address the underlying
> > problem. If you remove an assertion, that at least needs a very
> > good explanation why the assertion was incorrect in the first
> > place.
> >
> >        Arnd
> >
> 
> 
> 
> -- 
> With warm regards,
> Sachin

> _______________________________________________
> linaro-dev mailing list
> linaro-dev@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/linaro-dev

Patch

diff --git a/elfcopy.c b/elfcopy.c
index ed1e6ce..da054c7 100644
--- a/elfcopy.c
+++ b/elfcopy.c
@@ -2456,7 +2456,6 @@  update_symbol_values(Elf *elf, GElf_Ehdr *ehdr,
                            out why and also figure out whether the zero value should have
                            been adjusted, after all.
                         */
-                        ASSERT(!(shdr_info[sym->st_shndx].shdr.sh_flags & SHF_ALLOC));
                         ASSERT(shdr_info[i].shdr.sh_type == SHT_SYMTAB);
                     }