- User Since
- Jan 31 2017, 4:46 PM (89 w, 2 d)
Wed, Oct 17
Wed, Oct 3
The motivating use case always pairs noderef with an address_space attribute, and it's already invalid to convert between pointer types in different address spaces without a cast.
So I don't think we have a strong opinion one way or the other. In the abstract, I'd say noderef feels like a "constraining" qualifier a la const/volatile so that going from unconstrained to constrained implicitly is OK but not vice versa.
Fri, Sep 28
Wed, Sep 26
Sun, Sep 23
lgtm as long as we revert when the underlying bugs are resovled
Wed, Sep 19
Sep 18 2018
Sep 17 2018
The test will need to verify that the emitted relocs are correct too.
Sep 16 2018
lgtm. Note that most sanitizer runtime ports for Fuchsia will be DSO-only, but I think the #ifndef PIC condition will cover that if/when those cases come up.
For always-static runtimes that can't be statically linked into DSOs (i.e. runtime library is not included in -shared links) this should now be right for Fuchsia.
IIRC profile is always-static but meant to be statically linked separately into each DSO, but presumably that means it's built with PIC defined too.
Sep 14 2018
Sep 11 2018
Sep 8 2018
Sep 4 2018
Sep 1 2018
The log message could give concrete examples.
Aug 26 2018
Aug 1 2018
I thought the plan was to produce JSON, not YAML.
Jul 27 2018
I think this wants to be a hard error rather than a warning. Though since we use -Werror anyway if others feel strongly contrary I won't object.
Jul 23 2018
Jul 14 2018
Is this still live? Should it be different after all the multiarch stuff?
LGTM with these few substantive nits and then a whole lot more comments (mostly a more thorough overview covering linkage and output model).
Jul 10 2018
Almost there, but some nits.
Jun 26 2018
Jun 18 2018
Jun 15 2018
Jun 11 2018
In response to Jake's comments about GNUStyle:
Jun 7 2018
Jun 5 2018
Jun 4 2018
Jun 1 2018
lgtm modulo unnecessary #include
May 30 2018
I don't have enough context on how the profiling runtime works to properly review this implementation strategy for Fuchsia. Perhaps adequate comments here would tell me enough. Certainly it needs a lot more comments.
But probably we just need to talk through the implementation plan in person first.
May 23 2018
May 22 2018
May 21 2018
May 9 2018
LGTM assuming the Fuchsia build still works (I don't see any problems in the source off hand).
@jakehehrlich will verify the Fuchsia Build.
May 4 2018
@jakehehrlich is working on an open-source tool at https://fuchsia.googlesource.com/tools that handles this output format as specified in https://fuchsia.googlesource.com/zircon/+/master/docs/symbolizer_markup.md
I'd prefer to keep all the markup string constants together. Moving them all to a header would be fine, but I think it should be a separate sanitizer_symbolizer_fuchsia.h so that non-Fuchsia builds aren't using symbolizer_fuchsia.h, which is about OS-specific stuff distinct from the symbolizer markup format. But nothing in this patch actually needs the constant to be visible outside sanitizer_symbolizer_fuchsia.cc, so I'd like to see some explanation of the need for that.
May 2 2018
lgtm with a comment
May 1 2018
Apr 27 2018
I don't know the backend or the register allocator at all but this looks like a thorough parallel of the existing model for -ffixed-x18.
Apr 25 2018
Apr 24 2018
Apr 19 2018
We should revisit the implicit dependency on fdio. For other compiler-rt libraries' Fuchsia ports, we've kept it at the pure vDSO and minimal libc ABIs.
Apr 11 2018
Mar 30 2018
Mar 27 2018
lgtm but I'm not authoritative.
Mar 22 2018
Mar 20 2018
Lgtm with some more comments
When targetting Darwin we should really not be even compiling libc++/libc++abi and certainly should not have anything else compiling against their headers since at runtime the system libraries will be used and so things better compile against the system headers that match.
Do these settings accomplish that?
Mar 18 2018
We aren't actually using DWARF 5 yet AFAICT. Our builds don't pass -gdwarf-5. So I'm not sure we have yet verified that all the DWARF-consuming tools people are using with Fuchsia binaries can handle all of DWARF 5 (which has several major format changes). I'd certainly like 5 to be the default, but I think we need to establish a set of consumers we care about and verify their format version support before we can be sure about this.
Mar 14 2018
So Sam and I had dinner together. :-) I don't know a lot about WebAssembly and Sam doesn't know a lot about traditional (e.g. ELF) linkers, so we talked about the parallels in somewhat general terms while explaining the specifics to each other without distracting from our dessert.
In talking about what WebAssembly wants to do with converting some kinds of references to other kinds, I described the obvious analogy to the various kinds of linker relaxation that have been done in traditional machine code linking such as ELF.
There are two precedents that I can cite in detail off hand. In both cases the reloc type is specified in the ABI as for use with certain instruction patterns and the linker is expected to rewrite instructions with complete knowledge of their meaning, rather than just deliver values into bitfields.
One is x86-64's GOTPCREL and GOTPCRELX relocs, where GNU linkers check for expected opcode bytes immediate prior to the r_offset location and silently forego the instruction-rewriting optimization if the reloc is not used in the expected context.
The other is x86-64's TLS relocs, where GNU linkers check the opcode bytes immediatley prior to the r_offset location and diagnose an error if they don't match specific instruction patterns prescribed in the ABI for each reloc that enables linker relaxation (IIRC for some cases the x86 ABI mandates that linkers perform the relaxation, rather than just describing how the optimization is possible).
I don't know off hand how LLD handles these cases, but I would say that matching the GNU linkers' behavior is right--with the possible caveat that perhaps there should be a warning emitting for using GOTPCRELX relocs with instructions the linker doesn't know how to rewrite.
I know that other machines have many more relocs that are prescribed in their ABIs as for use with specific instruction patterns, but I don't know off hand what the details of those are, nor how existing linkers handle situations where such relocs are used with mismatching instructions.
Mar 5 2018
Mar 1 2018
Jan 12 2018
I wonder if cpplint.py is worthwhile any more. Perhaps it should be replaced by a check that clang-format doesn't decide to change anything.
clang-format and lib/sanitizer_common/scripts/cpplint.py disagree about whitespace before braces.
ping. Will this or D41277 land?
Dec 21 2017
Dec 20 2017
This works fine for me. Perhaps Rui should land D39348 first and then you can rebase this in terms of that to discuss the remaining features (-R alias, no input files case).
Dec 19 2017
Mutual exclusion is not target-specific and is already handled in SanitizerArgs. The test coverage for that looks fine.
Dec 18 2017
Works for me. D41277 works just as well. I don't know how you plan to reconcile the two.
Dec 1 2017
Great! Please land it for me.
Nov 30 2017
This change as is fixes my real-world case.
Thanks! Now I'm just blocked on --just-symbols...
Fuchsia bit LGTM. Linux bit highly suspect.
Nov 27 2017
Nov 21 2017
Nov 20 2017
Nov 17 2017
LGTM with comment added but if a different approach will be required for multiple Linux targets (which we all treat as "hostish") then it's worth at least thinking about that now.
LGTM. I wonder if it's really worth keeping --strip-non-alloc.
Nov 10 2017
In principle this should be OK since we're unconditionally building the runtimes for default.
However, this means that any dynamically-linked host builds will assume our toolchain-provided shared libraries are available on the runtime host in standard library places.
I don't think we want that. Perhaps we could make -static-libstdc++ default in our host targets. But that's not really enough, since compiler-rt has some too. Probably better to just disable building the shared libraries altogether for the host targets.
Oct 26 2017
This doesn't implement the -R switch name, which is the traditional interface from time immemorial. I can use --just-symbols, but you really should handle -R too.
Oct 24 2017
Oct 23 2017
This seems like how it always should have been, since -fuse-ld=lld sets it to ld.lld, not lld.
Looks to me like the existing MipsLinux and WebAssembly implementations are wrong too and only BareMetal is right.
Oct 21 2017
This change works on my case, but it names the reloc sections it emits based on the first input section in the output section being relocated rather than on the output section itself.
e.g. in my -ffunction-sections --gc-sections --emit-relocs build, the SHT_RELA section with sh_info pointing to the .text section is called .rela.text.first_function_in_text rather than .rela.text.
Oct 17 2017
Since we use lld, -Wl,-O2 also helps make the binaries smaller.
Oct 15 2017
http://www.sco.com/developers/gabi/latest/ch5.pheader.html#note_section describes alignment of the contents of notes. Maintaining that alignment is obviously impossible if the section is not so aligned.
Oct 13 2017
Oct 11 2017
Oct 10 2017
Oct 9 2017
Oct 8 2017
I wonder why my builds have never had an undefined symbol HandleDeadlySignal.