Summary: | www-client/seamonkey-2.49.5 BaseAssembler-x64.h directive argument is null | ||
---|---|---|---|
Product: | Gentoo Linux | Reporter: | raffaele <mentadent47> |
Component: | Current packages | Assignee: | Myckel Habets <gentoo-bugs> |
Status: | UNCONFIRMED --- | ||
Severity: | normal | CC: | atoth, gentoo-bugs, mozilla |
Priority: | Normal | ||
Version: | unspecified | ||
Hardware: | All | ||
OS: | Linux | ||
See Also: | https://bugs.gentoo.org/show_bug.cgi?id=685092 | ||
Whiteboard: | |||
Package list: | Runtime testing required: | --- | |
Attachments: | Build log |
Description
raffaele
2019-09-05 16:17:41 UTC
The problem originally occured on an XFCE-based box, if it matters. I have hit the same build problem on a different box, ~amd64 and LXDE-based. The XFCE one was installed from stage3 just a few weeks ago. On a third box, ~x86 and LXDE-based, the package builds without problems. I am switching to a different browser/email package combination so this problem has become less important for me but I can keep the package around for any checks that may help troubleshoot the issue, if necessary. Let me know. (In reply to raffaele from comment #1) > The problem originally occured on an XFCE-based box, if it matters. I have > hit the same build problem on a different box, ~amd64 and LXDE-based. The > XFCE one was installed from stage3 just a few weeks ago. On a third box, > ~x86 and LXDE-based, the package builds without problems. > > I am switching to a different browser/email package combination so this > problem has become less important for me but I can keep the package around > for any checks that may help troubleshoot the issue, if necessary. Let me > know. False positive. LSF simply disables Werror=format: http://www.linuxfromscratch.org/blfs/view/cvs/xsoft/seamonkey.html " GCC-9 generates some false positives with -Werror=format, which prevent building SeaMonkey. Remove this flag with the following command: grep -rl -- '-Werror=format' | xargs sed -i 's/error=format/no-&/' " Works for me. (In reply to Attila Tóth from comment #2) > LSF simply disables Werror=format: > GCC-9 generates some false positives I wouldn't call those "false positives". Passing NULL to a "%s" printf() format is a super-undefined behaviour. The C standard (I don't think C++ defines it but only refers to the C standard?) says that the argument should point to a character array. That's all, and NULL doesn't point to valid character array. Some compiler implementations happen to have chosen to have printf() print a specific string when they receive a NULL argument for "%s". But even those do it very erratically: On my setup with my gcc, printf("%s") will print '(null)'. But printf("%s\n") will segfault! Try this example: ------------------------ #include <stdio.h> int main(void) { printf("1: %s\n", NULL); printf("2: "); printf("%s", NULL); printf("\n"); printf("3: "); printf("%s\n", NULL); } ------------------------ With clang, it will print 3 '(null)' strings if you compile with -O0, but it will segfault on the third if you compile with -O1 (or more). With gcc, it always segfaults on the third. And with g++, it never segfaults. ################### Anyway, this sloppy stuff should be fixed upstream. As far as distros/users are concerned, I guess it is enough to assume that, considering the specific compilers used, and the specific cases involved, one may go with ignoring the undefined behaviour error/warning. NB: I am not a maintainer, this is just my opinion :-) (In reply to stephane.goujet from comment #3) > (In reply to Attila Tóth from comment #2) > > > > LSF simply disables Werror=format: > > GCC-9 generates some false positives > > I wouldn't call those "false positives". Passing NULL to a "%s" printf() > format is a super-undefined behaviour. > > The C standard (I don't think C++ defines it but only refers to the C > standard?) says that the argument should point to a character array. That's > all, and NULL doesn't point to valid character array. > > Some compiler implementations happen to have chosen to have printf() print a > specific string when they receive a NULL argument for "%s". But even those > do it very erratically: > > On my setup with my gcc, printf("%s") will print '(null)'. But > printf("%s\n") will segfault! > > > Try this example: > ------------------------ > #include <stdio.h> > > int main(void) { > printf("1: %s\n", NULL); > printf("2: "); > printf("%s", NULL); > printf("\n"); > printf("3: "); > printf("%s\n", NULL); > } > ------------------------ > > With clang, it will print 3 '(null)' strings if you compile with -O0, but it > will segfault on the third if you compile with -O1 (or more). With gcc, it > always segfaults on the third. And with g++, it never segfaults. > > ################### > > Anyway, this sloppy stuff should be fixed upstream. As far as distros/users > are concerned, I guess it is enough to assume that, considering the specific > compilers used, and the specific cases involved, one may go with ignoring > the undefined behaviour error/warning. > > NB: I am not a maintainer, this is just my opinion :-) Thanks. However I tried to modify the code to check the imput parameters before, but the compiler still bails out. Or I tried to do it wrong... (In reply to Attila Tóth from comment #4) > However I tried to modify the code to check the imput parameters before, but > the compiler still bails out. Or I tried to do it wrong... Hmmm... Am I right if I suppose you tested the last parameter (GP...) in that line? -------------------- spew("movq " MEM_obs ", %s", ADDR_obs(offset, base, index, scale), GPReg64Name(dst)) -------------------- If that's the case, I would say we are not looking at the right "%s". There is the obvious one, but there are also several formats hidden in the MEM_obs macro :-( MEM_obs is defined (in "mozilla/js/src/jit/x86-shared/AssemblerBuffer-x86-shared.h") as: -------------------- #define MEM_obs MEM_o "(%s,%s,%d)" -------------------- and MEM_o itself is defined -------------------- #define MEM_o "%s0x%x" -------------------- So that's a total of 3 extra "%s" which can fail. The final format is something like "movq %s0x%x(%s,%s,%d) %s". And they are fed by the ADDR_obs() macro, defined in the same file. I guess that's in this macro (and the one it calls) that tests for NULL should be inserted. I don't have time at the moment to investigate further, sorry. (And I also don't have gcc-9 on my setup to test anything + I have other problems compiling this damn version of semaonkey, so maybe I don't even manage to get compilation to the point of that error ;-) ). (In reply to stephane.goujet from comment #5) > (In reply to Attila Tóth from comment #4) > > However I tried to modify the code to check the imput parameters before, but > > the compiler still bails out. Or I tried to do it wrong... > > Hmmm... > > Am I right if I suppose you tested the last parameter (GP...) in that line? > > -------------------- > spew("movq " MEM_obs ", %s", ADDR_obs(offset, base, index, scale), > GPReg64Name(dst)) > -------------------- > > If that's the case, I would say we are not looking at the right "%s". There > is the obvious one, but there are also several formats hidden in the MEM_obs > macro :-( > > MEM_obs is defined (in > "mozilla/js/src/jit/x86-shared/AssemblerBuffer-x86-shared.h") as: > > -------------------- > #define MEM_obs MEM_o "(%s,%s,%d)" > -------------------- > > and MEM_o itself is defined > > -------------------- > #define MEM_o "%s0x%x" > -------------------- > > So that's a total of 3 extra "%s" which can fail. The final format is > something like "movq %s0x%x(%s,%s,%d) %s". > > And they are fed by the ADDR_obs() macro, defined in the same file. I guess > that's in this macro (and the one it calls) that tests for NULL should be > inserted. > > I don't have time at the moment to investigate further, sorry. (And I also > don't have gcc-9 on my setup to test anything + I have other problems > compiling this damn version of semaonkey, so maybe I don't even manage to > get compilation to the point of that error ;-) ). I also tried to track down those sequence of macros, creating a rabbit-hole. I'm not convinced those macros are helping us. I tried multiple parameters and may have failed to do that properly. It's also interesting that there are more functions similar to this one from regards of the parameters. Why only this one bothers the compiler? I also don't have enough time to get to the end of this. Additionally, I don't like the concept, that the user cannot choose whether to enable jit support. Older versions of mozilla stuff had a configuration option to do that... (In reply to Attila Tóth from comment #6) > I also tried to track down those sequence of macros, creating a rabbit-hole. Ok, so I was wrong, you didn't make the same mistake I would have made. Eh eh. > It's also interesting that there > are more functions similar to this one from regards of the parameters. Why > only this one bothers the compiler? It's strange. The 1st argument matching a "%s" is coming from PRETTYHEX and resolves to a string literal, and the other 3 seem to resolve to calls to GPReg64Name, which just return one element of an array of string literals, and this array contains no NULL. So I don't see where any NULL could come from. Could you try protecting the access to 'names' in "mozilla/js/src/jit/x86-shared/Constants-x86-shared.h" from a theoretical out-of-bounds index? Something like this: ------------------ inline const char* GPReg64Name(RegisterID reg) { static const char* const names[] = { "%rax", "%rcx", "%rdx", "%rbx", "%rsp", "%rbp", "%rsi", "%rdi" #ifdef JS_CODEGEN_X64 ,"%r8", "%r9", "%r10", "%r11", "%r12", "%r13", "%r14", "%r15" #endif }; MOZ_ASSERT(size_t(reg) < mozilla::ArrayLength(names)); /* return names[reg]; */ if(reg<invalid_reg) { return names[reg]; } else { return "boo!" } } ------------------ Perhaps it will please GCC. Just a thought. Going through old bug reports for www-client/seamonkey. The package in question has left the ebuild tree already for quite some time. Is this still an issue? |