Removing false positive warnings related to CVE-2021-42694 from the binutils code base

[Richard Earnshaw]

On 11/06/2025 18:21, Matthieu Longo wrote:
> On 2025-06-11 16:15, Matthieu Longo wrote:
>> Hi,
>>
>> Due to CVE-2021-42694 (see [1] and [2] for more details), some tools 
>> warn whenever non-ASCII characters appear in a source file.
>> Those warnings aim at highlighting a potential avenue of surreptitious 
>> source code alteration, relying on the usage of confusable Unicode 
>> characters.
>>
>> In my opinion, instances that are generating false positives (e.g. in 
>> [3]) should be removed, unless the presence of such characters is 
>> necessary for the job at hand. Indeed, keeping unneeded exceptions 
>> causes a risk of missing a genuine warning due to the noise caused by 
>> the false positives.
>>
>> At the same time, removing all Unicode characters from comments can be 
>> limiting. See e.g. the use of non-ASCII Σ in [4] which is used simply 
>> to stay in sync with the respective documentation.
>>
>> Given the conflicting points of view on the topic, I am looking for 
>> guidance from the binutils maintainers.
>>
>> Regards,
>> Matthieu
>>
>>
>> [1]: LWN article: https://lwn.net/Articles/874951/
>> [2]: Associated paper: https://trojansource.codes/trojan-source.pdf
>> [3]: https://inbox.sourceware.org/binutils/ 
>> de7b5208-8d29-494b-8d4c-78e1302e37f4@arm.com/T/ 
>> #m511a13422a67a6b6c4fdfa5ed62597191c0c281c
>> [4]: https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/tests/ 
>> x86_emulator/simd-sha.c;hb=refs/heads/staging
> 
> I tried to find others usages in binutils by compiling the code base 
> using -finput-charset=ASCII.
> 
> I would classify the non-ascii symbols in the following categories:
> (1) commonly used symbols in documentation which are not always 
> replaceable by ASCII equivalent: §, ‘, ’, –, etc.
> (2) (extended) latin characters with diacritics for the 
> contributors'name from either countries using the latin alphabet, or a 
> romanized version of their name.
> (3) math symbols, which might include more than math symbols, i.e. 
> greek, cyrillic, arabic letters: μ, Σ, ℤ, etc. China and Japan seems to 
> stick with the mix of greek and latin letters.
> 
> One category (4) that is missing in binutils but which I think might 
> occur in some code bases is emojis.
> 
> Categories (2) and (3) look very legitimate for an English code base.
> Category (1) should be ok, even if it might be better to avoid things 
> like – (a.k.a en dash) which is very confusing. The curly versions of 
> quotes or apostrophe seems acceptable, but there is a risk of 
> inconsistency across the code base if some people use them whereas 
> others not. § is certainly very useful, mostly to make reference to 
> specs or documentation.
> (4) does not seem relevant for binutils's code base.
> 
> I am not sure what a solution would look like here, since some of those 
> characters are 100% legitimate, and there is no way to enforce a 
> specific category with the existing values for -finput-charset.
> 
> NB: The patch below is not a candidate for merging, but only a reference 
> for the discussion.
> 
> Regards,
> Matthieu

For what they're worth, my 2c on this is that we should only do it if 
there's some mechanism for enforcing it, either during a git push that 
would reject invalid chars (it would need to be possible to configure 
exceptions for certain files or directories), or by an option to the 
compiler to limit the charset (eg -finput-charset=ASCII).  The latter 
might need to be applied per directory - I'm not sure we could enforce 
it for gnulib, for example.

R.
> 
> diff --git a/bfd/elf-attrs.c b/bfd/elf-attrs.c
> index bc653d780de..b4a57f4803c 100644
> --- a/bfd/elf-attrs.c
> +++ b/bfd/elf-attrs.c
> @@ -104,7 +104,7 @@ vendor_obj_attrs_v1_size (bfd *abfd, int vendor)
>          list = list->next)
>       size += obj_attr_v1_size (list->tag, &list->attr);
> 
> -  /* <size> <vendor_name> NUL 0x1 <size> */
> +  /* <size> <vendor_name> NUL 0x1 <size>  */
>     return (size
>         ? size + 10 + strlen (vendor_name)
>         : 0);
> @@ -117,7 +117,7 @@ bfd_elf_obj_attrs_v1_size (bfd *abfd)
>     size = vendor_obj_attrs_v1_size (abfd, OBJ_ATTR_PROC);
>     size += vendor_obj_attrs_v1_size (abfd, OBJ_ATTR_GNU);
>     if (size > 0)
> -    size += sizeof(uint8_t); /* <format-version: ‘A’>  */
> +    size += sizeof(uint8_t); /* <format-version: 'A'>  */
>     return size;
>   }
> 
> @@ -212,7 +212,7 @@ write_obj_attr_section_v1 (bfd *abfd, bfd_byte 
> *buffer, bfd_vma size)
>   {
>     bfd_byte *p = buffer;
> 
> -  /* <format-version: ‘A’>  */
> +  /* <format-version: 'A'>  */
>     *(p++) = 'A';
> 
>     for (int vendor = OBJ_ATTR_FIRST; vendor <= OBJ_ATTR_LAST; ++vendor)
> diff --git a/gas/gen-sframe.c b/gas/gen-sframe.c
> index 2d1c1961fd7..052dcc39501 100644
> --- a/gas/gen-sframe.c
> +++ b/gas/gen-sframe.c
> @@ -1511,7 +1511,7 @@ sframe_xlate_do_cfi_escape (const struct 
> sframe_xlate_ctx *xlate_ctx,
>   /* Translate DW_CFA_undefined into SFrame context.
> 
>      DW_CFA_undefined op indicates that from now on, the previous value of
> -   register can’t be restored anymore.  In SFrame stack trace, we cannot
> +   register can't be restored anymore.  In SFrame stack trace, we cannot
>      represent such a semantic.  So, we skip generating an SFrame FDE 
> for this,
>      when a register of interest is used with DW_CFA_undefined.
> 
> diff --git a/gnulib/config.in b/gnulib/config.in
> index f70c6a52b43..1877fd4e0e0 100644
> --- a/gnulib/config.in
> +++ b/gnulib/config.in
> @@ -1973,11 +1973,11 @@
>        1  if n1 > n2
>        0  if n1 == n2
>        -1 if n1 < n2
> -   The naïve code   (n1 > n2 ? 1 : n1 < n2 ? -1 : 0)  produces a 
> conditional
> +   The naive code   (n1 > n2 ? 1 : n1 < n2 ? -1 : 0)  produces a 
> conditional
>      jump with nearly all GCC versions up to GCC 10.
>      This variant     (n1 < n2 ? -1 : n1 > n2)  produces a conditional 
> with many
>      GCC versions up to GCC 9.
> -   The better code  (n1 > n2) - (n1 < n2)  from Hacker's Delight § 2-9
> +   The better code  (n1 > n2) - (n1 < n2)  from Hacker's Delight, 
> paragraph 2-9
>      avoids conditional jumps in all GCC versions >= 3.4.  */
>   #define _GL_CMP(n1, n2) (((n1) > (n2)) - ((n1) < (n2)))
> 
> diff --git a/gnulib/import/getprogname.c b/gnulib/import/getprogname.c
> index 62a480046e9..0d8bf7fdb0d 100644
> --- a/gnulib/import/getprogname.c
> +++ b/gnulib/import/getprogname.c
> @@ -94,7 +94,7 @@ getprogname (void)
>     return p && p[0] ? p : "?";
>   #  endif
>   # elif _AIX                                                 /* AIX */
> -  /* Idea by Bastien ROUCARIÈS,
> +  /* Idea by Bastien ROUCARIES,
>        https://lists.gnu.org/r/bug-gnulib/2010-12/msg00095.html
>        Reference: https://www.ibm.com/support/knowledgecenter/en/ 
> ssw_aix_61/com.ibm.aix.basetrf1/getprocs.htm
>     */
> diff --git a/gnulib/import/idx.h b/gnulib/import/idx.h
> index c3669ddaca8..b604efc9ad0 100644
> --- a/gnulib/import/idx.h
> +++ b/gnulib/import/idx.h
> @@ -36,7 +36,7 @@
>        * Security: Signed types can be checked for overflow via
>          '-fsanitize=undefined', but unsigned types cannot.
> 
> -     * Comparisons without surprises: ISO C99 § 6.3.1.8 specifies a few
> +     * Comparisons without surprises: ISO C99 paragraph 6.3.1.8 
> specifies a few
>          surprising results for comparisons, such as
> 
>              (int) -3 < (unsigned long) 7  =>  false
> @@ -45,7 +45,7 @@
>              (long) -3 < (unsigned int) 7  =>  false
> 
>          This is surprising because the natural comparison order is by
> -       value in the realm of infinite-precision signed integers (ℤ).
> +       value in the realm of infinite-precision signed integers (Z).
> 
>          The best way to get rid of such surprises is to use signed types
>          for numerical integer values, and use unsigned types only for
> diff --git a/gnulib/import/malloca.c b/gnulib/import/malloca.c
> index e7beaaf066e..0190d15f4cf 100644
> --- a/gnulib/import/malloca.c
> +++ b/gnulib/import/malloca.c
> @@ -29,10 +29,10 @@
>      result: it must be fast, to match the speed of alloca().  The speed of
>      mmalloca() and freea() in the other case are not critical, because 
> they
>      are only invoked for big memory sizes.
> -   Here we use a bit in the address as an indicator, an idea by Ondřej 
> Bílka.
> +   Here we use a bit in the address as an indicator, an idea by Ondrej 
> Bilka.
>      malloca() can return three types of pointers:
> -     - Pointers ≡ 0 mod 2*sa_alignment_max come from stack allocation.
> -     - Pointers ≡ sa_alignment_max mod 2*sa_alignment_max come from heap
> +     - Pointers = 0 mod 2*sa_alignment_max come from stack allocation.
> +     - Pointers = sa_alignment_max mod 2*sa_alignment_max come from heap
>          allocation.
>        - NULL comes from a failed heap allocation.  */
> 
> @@ -46,7 +46,7 @@ mmalloca (size_t n)
>   {
>   #if HAVE_ALLOCA
>     /* Allocate one more word, used to determine the address to pass to 
> freea(),
> -     and room for the alignment ≡ sa_alignment_max mod 
> 2*sa_alignment_max.  */
> +     and room for the alignment = sa_alignment_max mod 
> 2*sa_alignment_max.  */
>     uintptr_t alignment2_mask = 2 * sa_alignment_max - 1;
>     int plus = sizeof (small_t) + alignment2_mask;
>     idx_t nplus;
> @@ -71,7 +71,7 @@ mmalloca (size_t n)
>                So, the memory range [p, p+n) lies in the allocated 
> memory range
>                [mem, mem + nplus).  */
>             p[-1] = offset;
> -          /* p ≡ sa_alignment_max mod 2*sa_alignment_max.  */
> +          /* p = sa_alignment_max mod 2*sa_alignment_max.  */
>             return p;
>           }
>       }
> diff --git a/include/elf/common.h b/include/elf/common.h
> index fd032d1e03e..5064931ce89 100644
> --- a/include/elf/common.h
> +++ b/include/elf/common.h
> @@ -344,7 +344,7 @@
>   #define EM_LANAI    244    /* Lanai 32-bit processor.  */
>   #define EM_CEVA        245    /* CEVA Processor Architecture Family */
>   #define EM_CEVA_X2    246    /* CEVA X2 Processor Family */
> -#define EM_BPF        247    /* Linux BPF – in-kernel virtual machine.  */
> +#define EM_BPF        247    /* Linux BPF - in-kernel virtual machine.  */
>   #define EM_GRAPHCORE_IPU 248    /* Graphcore Intelligent Processing 
> Unit */
>   #define EM_IMG1        249    /* Imagination Technologies */
>   #define EM_NFP        250    /* Netronome Flow Processor.  */
> diff --git a/opcodes/i386-dis.c b/opcodes/i386-dis.c
> index 703f09dbb77..4773a829a4e 100644
> --- a/opcodes/i386-dis.c
> +++ b/opcodes/i386-dis.c
> @@ -10063,7 +10063,7 @@ print_insn (bfd_vma pc, disassemble_info *info, 
> int intel_syntax)
>         goto out;
>       }
> 
> -      /* EVEX from legacy instructions require that EVEX.z, EVEX.L’L 
> and the
> +      /* EVEX from legacy instructions require that EVEX.z, EVEX.L'L 
> and the
>        lower 2 bits of EVEX.aaa must be 0.  */
>         if ((ins.vex.mask_register_specifier & 0x3) != 0
>         || ins.vex.ll != 0 || ins.vex.zeroing != 0)
> diff --git a/sim/common/dv-cfi.c b/sim/common/dv-cfi.c
> index df6942d09c4..ed9e8274315 100644
> --- a/sim/common/dv-cfi.c
> +++ b/sim/common/dv-cfi.c
> @@ -572,8 +572,8 @@ cfi_add_erase_region (struct hw *me, struct cfi *cfi,
>          write_size: 0 (not supported)
>          erase_region: 1 (can only erase whole chip)
>          voltage: 0.0V (for all)
> -       timeouts: typ: 1µs, not supported, 1ms, not supported
> -                 max: 1µs, 1ms, 1ms, not supported
> +       timeouts: typ: 1us, not supported, 1ms, not supported
> +                 max: 1us, 1ms, 1ms, not supported
> 
>     TODO: Verify user args are valid (e.g. voltage is 8 bits).  */
>   static void
> 
>