Removing false positive warnings related to CVE-2021-42694 from the binutils code base
Matthieu Longo
matthieu.longo@arm.com
Wed Jun 11 17:21:51 GMT 2025
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
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
More information about the Binutils
mailing list