Messages in this thread | | | From | Fāng-ruì Sòng <> | Date | Fri, 22 Jan 2021 12:36:03 -0800 | Subject | Re: [PATCH] checkpatch: add warning for avoiding .L prefix symbols in assembly files |
| |
On Fri, Jan 22, 2021 at 12:13 PM Aditya <yashsri421@gmail.com> wrote: > > On 23/1/21 12:40 am, Joe Perches wrote: > > On Fri, 2021-01-22 at 18:48 +0530, Aditya wrote: > >> On 21/1/21 12:13 am, Joe Perches wrote: > >>> I believe the test should be: > >>> > >>> if ($realfile =~ /\.S$/ && > >>> $line =~ /^\+\s*SYM_[A-Z]+_(?:START|END)(?:_[A-Z_]+)?\s*\(\s*\.L/) { > >> > >> Joe, with this regex, we won't be able to match > >> "jmp .L_restore > >> SYM_FUNC_END(\name)" > > > > I think that's not an issue. > > > >> which was replaced in this patch in the discussion: > >> https://groups.google.com/g/clang-built-linux/c/-drkmLgu-cU/m/phKe-Tb6CgAJ > >> > >> Here, "jmp .L_restore" was also replaced to fix the error. > > > > Notice that this line was also replaced in the same patch: > > > > #ifdef CONFIG_PREEMPTION > > -SYM_CODE_START_LOCAL_NOALIGN(.L_restore) > > +SYM_CODE_START_LOCAL_NOALIGN(__thunk_restore) > > > > > >> However, if this > >> regex(/^\+\s*SYM_[A-Z]+_(?:START|END)(?:_[A-Z_]+)?\s*\(\s*\.L/) is > >> correct, maybe we don't need to check for $file as we are now checking > >> for just one line. > >> > >> What do you think? > > > > The test I wrote was complete, did not use $file and emits a > > warning on the use of CODE_SYM_START_LOCAL_NOALIGN(.L_restore) > > > > I think it's sufficient. > > > > Okay, Thanks.. I will send the modified patch :) > > Thanks > Aditya
I am slightly concerned with the direction here. jmp .Lsym is fine and is probably preferable in cases where you absolutely want to emit a local symbol. (there is a related -z unique-symbol GNU ld+FGKASLR thing I shall analyze in my spare time)
If the problem is just that STT_SECTION symbols are missing, can objtool do something to conceptually synthesize them?
| |