| Message ID | mcrwqklf6rs.fsf@iant-glaptop.roam.corp.google.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Nov 6, 2013 at 4:03 PM, Bruce Korb <bruce.korb@gmail.com> wrote: > please try to see if just one file name expression is sufficient. > If one is sufficient, please fix, otherwise, approved. > But, also, "all active branches", please. I tried to do that, but I ran into my lack of knowledge of the fixincludes framework. I need to recognize bits/fenv.h and x86_64-linux-gnu/bits/fenv.h. I can't use only "*/bits/fenv.h" because the docs say that the first file should not start with a wildcard character. Can you suggest an approach that may work? Ian
On 11/06/13 15:29, Ian Lance Taylor wrote: > When fenv.h is not fixed, libquadmath does not build. > > This patch works around the problem. Bootstrapped and tested on > x86_64-unknown-linux-gnu. > > OK for mainline? Hi Ian, Yes, please. This time, I'm on my dev box and looked at the code. You remembered correctly that the first file name in the list of file names needs to not have wild card characters so that the testing scheme can create a file by that name. A directory named "*" is possible, but inconvenient. Anyway, it also matches prior art in: > /* > * glibc_c99_inline_3 > */ > fix = { > hackname = glibc_c99_inline_3; > files = bits/string2.h, '*/bits/string2.h'; but is inconsistent with: > /* Some versions of glibc have a version of bits/string2.h that > produces "value computed is not used" warnings from strncpy; fix > this definition by using __builtin_strncpy instead as in newer > versions. */ > fix = { > hackname = glibc_strncpy; > files = bits/string2.h; Hmmm. Does that fix need fixing, too? Thanks -Bruce
So is this the right patch? > $ svn diff inclhack.def > Index: inclhack.def > =================================================================== > --- inclhack.def (revision 204533) > +++ inclhack.def (working copy) > @@ -1738,7 +1738,7 @@ > versions. */ > fix = { > hackname = glibc_strncpy; > - files = bits/string2.h; > + files = bits/string2.h, '*/bits/string2.h'; > bypass = "__builtin_strncpy"; > c_fix = format; > c_fix_arg = "# define strncpy(dest, src, n) __builtin_strncpy (dest, src, n)"; > @@ -2411,7 +2411,7 @@ > */ > fix = { > hackname = huge_val_hex; > - files = bits/huge_val.h; > + files = bits/huge_val.h, '*/bits/huge_val.h'; > select = "^#[ \t]*define[ \t]*HUGE_VAL[ \t].*0x1\\.0p.*"; > bypass = "__builtin_huge_val"; > > @@ -2426,7 +2426,7 @@ > */ > fix = { > hackname = huge_valf_hex; > - files = bits/huge_val.h; > + files = bits/huge_val.h, '*/bits/huge_val.h'; > select = "^#[ \t]*define[ \t]*HUGE_VALF[ \t].*0x1\\.0p.*"; > bypass = "__builtin_huge_valf"; > > @@ -2441,7 +2441,7 @@ > */ > fix = { > hackname = huge_vall_hex; > - files = bits/huge_val.h; > + files = bits/huge_val.h, '*/bits/huge_val.h'; > select = "^#[ \t]*define[ \t]*HUGE_VALL[ \t].*0x1\\.0p.*"; > bypass = "__builtin_huge_vall"; > > @@ -4226,7 +4226,7 @@ > fix = { > hackname = thread_keyword; > files = "pthread.h"; > - files = "bits/sigthread.h"; > + files = bits/sigthread.h, '*/bits/sigthread.h'; > select = "([* ])__thread([,)])"; > c_fix = format; > c_fix_arg = "%1__thr%2"; > @@ -4767,7 +4767,7 @@ > fix = { > hackname = feraiseexcept_nosse_invalid; > mach = 'i[34567]86-*-linux*', 'x86*-linux*', 'amd64-*-linux*'; > - files = bits/fenv.h; > + files = bits/fenv.h, '*/bits/fenv.h'; > select = "^([\t ]*)__asm__ __volatile__ \\(\"divss %0, %0 *\" : " > ": \"x\" \\(__f\\)\\);$"; > bypass = "\"fdiv .*; fwait\""; > @@ -4794,7 +4794,7 @@ > fix = { > hackname = feraiseexcept_nosse_divbyzero; > mach = 'i[34567]86-*-linux*', 'x86*-linux*', 'amd64-*-linux*'; > - files = bits/fenv.h; > + files = bits/fenv.h, '*/bits/fenv.h'; > select = "^([\t ]*)__asm__ __volatile__ \\(\"divss %1, %0 *\" : " > ": \"x\" \\(__f\\), \"x\" \\(__g\\)\\);$"; > bypass = "\"fdivp .*; fwait\"";
On Thu, Nov 7, 2013 at 8:48 AM, Bruce Korb <bkorb@gnu.org> wrote: > > This time, I'm on my dev box and looked at the code. > You remembered correctly that the first file name in the list > of file names needs to not have wild card characters so that > the testing scheme can create a file by that name. A directory > named "*" is possible, but inconvenient. > > Anyway, it also matches prior art in: > >> /* >> * glibc_c99_inline_3 >> */ >> fix = { >> hackname = glibc_c99_inline_3; >> files = bits/string2.h, '*/bits/string2.h'; > > > but is inconsistent with: > >> /* Some versions of glibc have a version of bits/string2.h that >> produces "value computed is not used" warnings from strncpy; fix >> this definition by using __builtin_strncpy instead as in newer >> versions. */ >> fix = { >> hackname = glibc_strncpy; >> files = bits/string2.h; > > > Hmmm. Does that fix need fixing, too? I didn't worry about that and the other cases because they are fixing problems that existed in past glibc versions before Ubuntu changed their directory layouts. But of course there is nothing wrong with fixing them too. Your proposed patch looks fine to me and I think you should go ahead and commit. Or I can if you prefer for some reason. Ian
In the meantime I've committed my version of the patch to the gccgo branch. It will be updated to whatever the final mainline version is the next time I merge from mainline to the gccgo branch. Ian On Thu, Nov 7, 2013 at 10:16 AM, Ian Lance Taylor <iant@google.com> wrote: > On Thu, Nov 7, 2013 at 8:48 AM, Bruce Korb <bkorb@gnu.org> wrote: >> >> This time, I'm on my dev box and looked at the code. >> You remembered correctly that the first file name in the list >> of file names needs to not have wild card characters so that >> the testing scheme can create a file by that name. A directory >> named "*" is possible, but inconvenient. >> >> Anyway, it also matches prior art in: >> >>> /* >>> * glibc_c99_inline_3 >>> */ >>> fix = { >>> hackname = glibc_c99_inline_3; >>> files = bits/string2.h, '*/bits/string2.h'; >> >> >> but is inconsistent with: >> >>> /* Some versions of glibc have a version of bits/string2.h that >>> produces "value computed is not used" warnings from strncpy; fix >>> this definition by using __builtin_strncpy instead as in newer >>> versions. */ >>> fix = { >>> hackname = glibc_strncpy; >>> files = bits/string2.h; >> >> >> Hmmm. Does that fix need fixing, too? > > I didn't worry about that and the other cases because they are fixing > problems that existed in past glibc versions before Ubuntu changed > their directory layouts. But of course there is nothing wrong with > fixing them too. > > Your proposed patch looks fine to me and I think you should go ahead > and commit. Or I can if you prefer for some reason. > > Ian
OK. It will be a couple of days. On Thu, Nov 7, 2013 at 1:01 PM, Ian Lance Taylor <iant@google.com> wrote: > In the meantime I've committed my version of the patch to the gccgo > branch. It will be updated to whatever the final mainline version is > the next time I merge from mainline to the gccgo branch. > > Ian > > On Thu, Nov 7, 2013 at 10:16 AM, Ian Lance Taylor <iant@google.com> wrote: >> On Thu, Nov 7, 2013 at 8:48 AM, Bruce Korb <bkorb@gnu.org> wrote: >>> >>> This time, I'm on my dev box and looked at the code. >>> You remembered correctly that the first file name in the list >>> of file names needs to not have wild card characters so that >>> the testing scheme can create a file by that name. A directory >>> named "*" is possible, but inconvenient. >>> >>> Anyway, it also matches prior art in: >>> >>>> /* >>>> * glibc_c99_inline_3 >>>> */ >>>> fix = { >>>> hackname = glibc_c99_inline_3; >>>> files = bits/string2.h, '*/bits/string2.h'; >>> >>> >>> but is inconsistent with: >>> >>>> /* Some versions of glibc have a version of bits/string2.h that >>>> produces "value computed is not used" warnings from strncpy; fix >>>> this definition by using __builtin_strncpy instead as in newer >>>> versions. */ >>>> fix = { >>>> hackname = glibc_strncpy; >>>> files = bits/string2.h; >>> >>> >>> Hmmm. Does that fix need fixing, too? >> >> I didn't worry about that and the other cases because they are fixing >> problems that existed in past glibc versions before Ubuntu changed >> their directory layouts. But of course there is nothing wrong with >> fixing them too. >> >> Your proposed patch looks fine to me and I think you should go ahead >> and commit. Or I can if you prefer for some reason. >> >> Ian
Index: inclhack.def =================================================================== --- inclhack.def (revision 204430) +++ inclhack.def (working copy) @@ -4768,6 +4768,7 @@ fix = { hackname = feraiseexcept_nosse_invalid; mach = 'i[34567]86-*-linux*', 'x86*-linux*', 'amd64-*-linux*'; files = bits/fenv.h; + files = "*/bits/fenv.h"; select = "^([\t ]*)__asm__ __volatile__ \\(\"divss %0, %0 *\" : " ": \"x\" \\(__f\\)\\);$"; bypass = "\"fdiv .*; fwait\""; @@ -4795,6 +4796,7 @@ fix = { hackname = feraiseexcept_nosse_divbyzero; mach = 'i[34567]86-*-linux*', 'x86*-linux*', 'amd64-*-linux*'; files = bits/fenv.h; + files = "*/bits/fenv.h"; select = "^([\t ]*)__asm__ __volatile__ \\(\"divss %1, %0 *\" : " ": \"x\" \\(__f\\), \"x\" \\(__g\\)\\);$"; bypass = "\"fdivp .*; fwait\"";