linux-kernel - Re: [PATCH -next] modpost: move some defines to the file head

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <mhng-e12b1787-9deb-487c-a595-2fb4c3f91178@palmer-ri-x1c9>
Date:   Fri, 21 Jul 2023 07:01:14 -0700 (PDT)
From:   Palmer Dabbelt <palmer@...osinc.com>
To:     masahiroy@...nel.org
CC:     wangkefeng.wang@...wei.com, mcgrof@...nel.org, nathan@...nel.org,
        ndesaulniers@...gle.com, nicolas@...sle.eu,
        linux-kbuild@...r.kernel.org, linux-kernel@...r.kernel.org
Subject:     Re: [PATCH -next] modpost: move some defines to the file head

On Fri, 21 Jul 2023 04:58:20 PDT (-0700), masahiroy@...nel.org wrote:
> On Thu, Jul 13, 2023 at 1:28 AM Palmer Dabbelt <palmer@...osinc.com> wrote:
>>
>> On Wed, 12 Jul 2023 08:55:23 PDT (-0700), masahiroy@...nel.org wrote:
>> > +To: Luis Chamberlain, the commiter of the breakage
>> >
>> >
>> >
>> > On Wed, Jul 12, 2023 at 10:44 AM Kefeng Wang <wangkefeng.wang@...wei.com> wrote:
>> >>
>> >> with "module: Ignore RISC-V mapping symbols too", build error occurs,
>> >>
>> >> scripts/mod/modpost.c: In function ‘is_valid_name’:
>> >> scripts/mod/modpost.c:1055:57: error: ‘EM_RISCV’ undeclared (first use in this function)
>> >>   return !is_mapping_symbol(name, elf->hdr->e_machine == EM_RISCV);
>> >>
>> >> Fix it by moving the EM_RISCV to the file head, also some other
>> >> defines in case of similar problem in the future.
>> >
>> >
>> >
>> > BTW, why is the flag 'is_riscv' needed?
>> >
>> >
>> > All symbols starting with '$' look special to me.
>> >
>> >
>> >
>> > Why not like this?
>> >
>> >
>> >        if (str[0] == '$')
>> >                  return true;
>> >
>> >        return false;
>>
>> There's a bit of commentary in the v1
>> <https://lore.kernel.org/all/20230707054007.32591-1-palmer@rivosinc.com/>,
>> but essentially it's not necessary.  I just wanted to play things safe
>> and avoid changing the mapping symbol detection elsewhere in order to
>> deal with RISC-V.
>>
>> IIRC we decided $ was special in RISC-V because there were some other
>> ports that behaved that way, but it wasn't universal.  If folks are OK
>> treating $-prefixed symbols as special everywhere that's fine with me, I
>> just wasn't sure what the right answer was.
>>
>> There's also some similar arch-specific-ness with the labels and such in
>> here.
>
> Hi Palmer,
>
> I am not a toolchain expert, but my gut feeling is
> that the code was safer than needed.
>
>
> I'd like to remove the 'is_riscv' switch rather than
> applying this patch.
>
> Will you send a patch, or do you want me to do so?

I've pretty much got it already.  Do you want it on top of the original 
patch, or just squashed in so you can drop it?