On Mon, May 11, 2015 at 11:17:39AM +0100, Russell King - ARM Linux wrote: > On Mon, May 11, 2015 at 10:56:57AM +0100, Dave P Martin wrote: > > ldr= will do the right thing *if* the target symbol's type is correctly > > annotated. This means that ldr =some_local_code_symbol does the right > > thing for branch target addresses if and only if some_local_code_symbol > > is marked with .type %function (or ENDPROC). > > > > The fact that a symbol is in a code section is *not* enough. > > > > For ARM code this never mattered, so local symbols in .S files are > > probably under-annotated in general. BSYM() might have been used to > > work around this in some cases. > > > > We should check that all the BSYMs removed by this series from ldr= > > and .long/.word etc. point to a correctly annotated symbol, and add > > the annotations if not. > > I guess the problem here is that people forget what a patch series is > actually doing and then start making comments based on hypothetical > stuff rather than what the series is actually doing. > > To recap, this series: > > 1. Removes the wrong usage of BSYM() in the KVM code. This is used with > the symbol "panic" which is a function declared by generic C code. > Therefore, panic will have the appropriate annotations attached to > this symbol, unless GCC itself is buggy. > > This is the _only_ case of "ldr rd, =BSYM(sym)". That's a sound conclusion -- you did mention before that that was the only BSYM with ldr=, but it slipped my mind again, sorry. > 2. It replaces the remaining "adr rd, BSYM(sym)" with a new "badr" macro, > and removes the BSYM() preprocessor macro. The new "badr" macro which > is identical in behaviour to the former, but ensures that BSYM() > doesn't get mis-used with "ldr". And this part wasn't in question anyway, so all is fine. Cheers ---Dave -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html