-Werror=stringop-* by alejandro-colomar · Pull Request #1401 · shadow-maint/shadow

alejandro-colomar · 2025-12-05T17:15:06Z

Enable some errors

Revisions:

v1b

Rebase

$ git rd 
1:  e2b8dea92 = 1:  914cc281d autogen.sh: Enable -Werror=stringop-* diagnostics
2:  6afa1de54 = 2:  54a9b2f25 tests/unit/test_strncpy.c: Remove strncpy_a() tests

v1c

Rebase

$ git rd 
1:  914cc281d = 1:  d80d40e9b autogen.sh: Enable -Werror=stringop-* diagnostics
2:  54a9b2f25 = 2:  5284b6830 tests/unit/test_strncpy.c: Remove strncpy_a() tests

v1d

Rebase

$ git rd 
1:  d80d40e9b289 = 1:  6fc3fff7ca98 autogen.sh: Enable -Werror=stringop-* diagnostics
2:  5284b68308cd = 2:  b881eee7e628 tests/unit/test_strncpy.c: Remove strncpy_a() tests

v1e

Rebase

$ git rd 
1:  6fc3fff7ca98 = 1:  989757d8b204 autogen.sh: Enable -Werror=stringop-* diagnostics
2:  b881eee7e628 ! 2:  692bb72f92b2 tests/unit/test_strncpy.c: Remove strncpy_a() tests
    @@ Commit message
     
      ## tests/unit/Makefile.am ##
     @@ tests/unit/Makefile.am: check_PROGRAMS = \
    -     test_atoi_strtoi \
    +     test_chkhash \
          test_chkname \
          test_snprintf \
     -    test_strncpy \

v2

Rebase
Drop -Werror=stringop-truncation. There's a false positive in legitimate strncpy(3) calls.

$ git rd 
1:  989757d8 ! 1:  a1c9e134 autogen.sh: Enable -Werror=stringop-* diagnostics
    @@ Commit message
         We can't enable -Wstringop-overread because it has bogus diagnostics
         with legitimate strncat(3) calls.
     
    +    We can't enable -Wstringop-truncation because it has bogus diagnostics
    +    with legitimate strncpy(3) calls.
    +
         Link: <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=123024>
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
     
    @@ autogen.sh: CFLAGS="$CFLAGS -Werror=incompatible-pointer-types"
      CFLAGS="$CFLAGS -Werror=sign-compare"
      CFLAGS="$CFLAGS -Werror=sizeof-pointer-div"
     +CFLAGS="$CFLAGS -Werror=stringop-overflow=4"
    -+CFLAGS="$CFLAGS -Werror=stringop-truncation"
      CFLAGS="$CFLAGS -Werror=unused-but-set-parameter"
      CFLAGS="$CFLAGS -Werror=unused-function"
      CFLAGS="$CFLAGS -Werror=unused-label"
2:  692bb72f = 2:  52dee45f tests/unit/test_strncpy.c: Remove strncpy_a() tests

v2b

Rebase

$ git rd -U0
1:  a1c9e134 = 1:  64df7a0f autogen.sh: Enable -Werror=stringop-* diagnostics
2:  52dee45f ! 2:  b848f9fe tests/unit/test_strncpy.c: Remove strncpy_a() tests
    @@ tests/unit/test_strncpy.c (deleted)
    +-#include "attr.h"
    @@ tests/unit/test_strncpy.c (deleted)
    --
    --static void test_strncpy_a_trunc(void **);
    --static void test_strncpy_a_fit(void **);
    --static void test_strncpy_a_pad(void **);
    +-static void test_strncpy_a_trunc(MAYBE_UNUSED void ** _1);
    +-static void test_strncpy_a_fit(MAYBE_UNUSED void ** _1);
    +-static void test_strncpy_a_pad(MAYBE_UNUSED void ** _1);
    @@ tests/unit/test_strncpy.c (deleted)
    --test_strncpy_a_trunc(void **)
    +-test_strncpy_a_trunc(MAYBE_UNUSED void ** _1)
    @@ tests/unit/test_strncpy.c (deleted)
    --test_strncpy_a_fit(void **)
    +-test_strncpy_a_fit(MAYBE_UNUSED void ** _1)
    @@ tests/unit/test_strncpy.c (deleted)
    --test_strncpy_a_pad(void **)
    +-test_strncpy_a_pad(MAYBE_UNUSED void ** _1)

v2c

Rebase

$ git rd 
1:  64df7a0f ! 1:  60baaf47 autogen.sh: Enable -Werror=stringop-* diagnostics
    @@ Commit message
      ## autogen.sh ##
     @@ autogen.sh: CFLAGS="$CFLAGS -Werror=incompatible-pointer-types"
      CFLAGS="$CFLAGS -Werror=int-conversion"
    - CFLAGS="$CFLAGS -Werror=sign-compare"
    + CFLAGS="$CFLAGS -Werror=overflow"
      CFLAGS="$CFLAGS -Werror=sizeof-pointer-div"
     +CFLAGS="$CFLAGS -Werror=stringop-overflow=4"
      CFLAGS="$CFLAGS -Werror=unused-but-set-parameter"
2:  b848f9fe = 2:  f1d1e2c4 tests/unit/test_strncpy.c: Remove strncpy_a() tests

v2d

Rebase

$ git rd 
1:  60baaf47 = 1:  759e4ed1 autogen.sh: Enable -Werror=stringop-* diagnostics
2:  f1d1e2c4 ! 2:  cc60b931 tests/unit/test_strncpy.c: Remove strncpy_a() tests
    @@ tests/unit/Makefile.am
     @@ tests/unit/Makefile.am: check_PROGRAMS = \
          test_chkhash \
          test_chkname \
    -     test_snprintf \
    +     test_stprintf \
     -    test_strncpy \
          test_strtcpy \
          test_typetraits \
          test_exit_if_null
    -@@ tests/unit/Makefile.am: test_snprintf_LDADD = \
    +@@ tests/unit/Makefile.am: test_stprintf_LDADD = \
          $(CMOCKA_LIBS) \
          $(NULL)

v2e

Rebase

$ git rd 
1:  759e4ed18f6d = 1:  339452eb8f11 autogen.sh: Enable -Werror=stringop-* diagnostics
2:  cc60b9319ed2 ! 2:  801b1ca131a8 tests/unit/test_strncpy.c: Remove strncpy_a() tests
    @@ tests/unit/Makefile.am: test_stprintf_LDADD = \
     -test_strncpy_LDFLAGS = \
     -    $(NULL)
     -test_strncpy_LDADD = \
    +-    $(LIBSHADOW) \
     -    $(CMOCKA_LIBS) \
     -    $(NULL)
     -
      test_strtcpy_SOURCES = \
    -     ../../lib/string/strcpy/strtcpy.c \
          test_strtcpy.c \
    +     $(NULL)
     
      ## tests/unit/test_strncpy.c (deleted) ##
     @@

v2f

Reviewed-by: @hallyn

$ git rd 
1:  339452eb8f11 ! 1:  38a078b8ecca autogen.sh: Enable -Werror=stringop-* diagnostics
    @@ Commit message
         with legitimate strncpy(3) calls.
     
         Link: <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=123024>
    +    Reviewed-by: Serge Hallyn <serge@hallyn.com>
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
     
      ## autogen.sh ##
2:  801b1ca131a8 ! 2:  d12f08cba686 tests/unit/test_strncpy.c: Remove strncpy_a() tests
    @@ Commit message
         arrays, but I never looked at that code.)
     
         Link: <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=122963>
    +    Reviewed-by: Serge Hallyn <serge@hallyn.com>
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
     
      ## tests/unit/Makefile.am ##

alejandro-colomar · 2025-12-05T17:46:47Z

The opensuse CI is consistently failing, but it seems to be a spurious error.

alejandro-colomar · 2026-01-08T02:33:01Z

Cc: @kees

kees

Hm, I never like to see tests removed unless an API has been removed with a compile-time check for its accidental reintroduction. Has strncpy been completely removed from this codebase? Perhaps add:

#define strncpy(...) static_assert(false, "Never use strncpy")

or something like?

alejandro-colomar · 2026-02-26T22:46:49Z

Hm, I never like to see tests removed unless an API has been removed with a compile-time check for its accidental reintroduction. Has strncpy been completely removed from this codebase? Perhaps add:
#define strncpy(...) static_assert(false, "Never use strncpy")
or something like?

No, we do actually use strncpy(3) through its wrapper strncpy_a(); it's a function I actually like (for a niche use case).

The reason for removing that test is not that we don't use it, but that GCC has false positives.

See this GCC report (from me):

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=122963

That report is about strncat(3), but strncpy(3) has a similar false positive.

alejandro-colomar · 2026-02-26T23:16:41Z

Also, I'm not too worried about the tests, because the implementation is so trivial that the tests are overkill.

#define strncpy_a(dst, src)  strncpy(dst, src, countof(dst))

We added the tests back when the API was significantly more complex, IIRC.

alejandro-colomar · 2026-06-20T21:21:17Z

@hallyn

hallyn

Sounds reasonable. I trust your judgement on this.

hallyn · 2026-07-15T14:38:05Z

Oh phoey - pls ping me when you resolve the conflict :)

(or reply and I'll do it)

alejandro-colomar · 2026-07-15T16:08:56Z

Oh phoey - pls ping me when you resolve the conflict :)

(or reply and I'll do it)

Solved (in v2e); thanks!

We can't enable -Wstringop-overread because it has bogus diagnostics with legitimate strncat(3) calls. We can't enable -Wstringop-truncation because it has bogus diagnostics with legitimate strncpy(3) calls. Link: <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=123024> Reviewed-by: Serge Hallyn <serge@hallyn.com> Signed-off-by: Alejandro Colomar <alx@kernel.org>

It's just the obvious thin wrapper around strncpy(3); let's trust it's ok. The reason for removing this test is that GCC has bogus diagnostics for strncpy(3). Its diagnostics are geared towards helping people that abuse strncpy(3) as a poor-man's strlcpy(3) not write exploitable code as easily. Using strncpy(3) for that purpose is brain damaged, and those programs should be audited to stop using this API for that. And most importantly, GCC should stop encouraging writing bad code that calls strncpy(3) as that results in diagnostics that are actively harmful for us, legitimate users of strncpy(3). Those false positives should certainly be out of -Wall. We could fill the tests with pragmas, but let's just remove the tests. I don't feel like maintaining code for ignoring GCC's brain bamage. If people want to write a function for truncating strings (because they can't rely on strlcpy(3) being available, or because they don't like it), they certainly should write such an API. strncpy(3) isn't that API. strncpy(3) is a function that takes a string and writes it into a utmpx(5) member, which is NOT a string. (And I heard it might also be useful for implementing tar(1), which also uses non-terminated character arrays, but I never looked at that code.) Link: <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=122963> Reviewed-by: Serge Hallyn <serge@hallyn.com> Signed-off-by: Alejandro Colomar <alx@kernel.org>

hallyn · 2026-07-15T16:11:12Z

Awesome, thanks.

alejandro-colomar force-pushed the stringop branch from b5d9a4e to 42d22a9 Compare December 5, 2025 17:16

alejandro-colomar marked this pull request as ready for review December 5, 2025 17:16

alejandro-colomar requested review from hallyn and ikerexxe December 5, 2025 17:16

alejandro-colomar force-pushed the stringop branch from 42d22a9 to 6afa1de Compare December 5, 2025 17:31

This was referenced Dec 5, 2025

-Werror=... #1220

Draft

4.19 Release plan (Herve) - 2025-12 #1354

Closed

alejandro-colomar force-pushed the stringop branch from 6afa1de to 54a9b2f Compare December 7, 2025 13:35

alejandro-colomar self-assigned this Dec 11, 2025

alejandro-colomar force-pushed the stringop branch from 54a9b2f to 5284b68 Compare December 31, 2025 02:11

alejandro-colomar mentioned this pull request Feb 13, 2026

4.20 Release plan (I) - 2026-06 #1451

Open

alejandro-colomar force-pushed the stringop branch 2 times, most recently from b881eee to 692bb72 Compare February 20, 2026 00:15

alejandro-colomar force-pushed the stringop branch from 692bb72 to 52dee45 Compare February 26, 2026 15:32

kees reviewed Feb 26, 2026

View reviewed changes

alejandro-colomar force-pushed the stringop branch from 52dee45 to b848f9f Compare March 4, 2026 18:25

alejandro-colomar force-pushed the stringop branch from b848f9f to f1d1e2c Compare April 12, 2026 08:47

alejandro-colomar force-pushed the stringop branch from f1d1e2c to cc60b93 Compare May 15, 2026 13:53

hallyn approved these changes Jul 15, 2026

View reviewed changes

alejandro-colomar force-pushed the stringop branch from cc60b93 to 801b1ca Compare July 15, 2026 16:08

alejandro-colomar added 2 commits July 15, 2026 18:09

alejandro-colomar force-pushed the stringop branch from 801b1ca to d12f08c Compare July 15, 2026 16:10

hallyn merged commit f02b40c into shadow-maint:master Jul 15, 2026
13 of 15 checks passed

alejandro-colomar deleted the stringop branch July 15, 2026 16:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

-Werror=stringop-*#1401

-Werror=stringop-*#1401
hallyn merged 2 commits into
shadow-maint:masterfrom
alejandro-colomar:stringop

alejandro-colomar commented Dec 5, 2025 •

edited

Loading

Uh oh!

alejandro-colomar commented Dec 5, 2025

Uh oh!

alejandro-colomar commented Jan 8, 2026

Uh oh!

kees left a comment

Uh oh!

alejandro-colomar commented Feb 26, 2026

Uh oh!

alejandro-colomar commented Feb 26, 2026 •

edited

Loading

Uh oh!

alejandro-colomar commented Jun 20, 2026

Uh oh!

hallyn left a comment

Uh oh!

hallyn commented Jul 15, 2026

Uh oh!

alejandro-colomar commented Jul 15, 2026 •

edited

Loading

Uh oh!

Uh oh!

hallyn commented Jul 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

alejandro-colomar commented Dec 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alejandro-colomar commented Dec 5, 2025

Uh oh!

alejandro-colomar commented Jan 8, 2026

Uh oh!

kees left a comment

Choose a reason for hiding this comment

Uh oh!

alejandro-colomar commented Feb 26, 2026

Uh oh!

alejandro-colomar commented Feb 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alejandro-colomar commented Jun 20, 2026

Uh oh!

hallyn left a comment

Choose a reason for hiding this comment

Uh oh!

hallyn commented Jul 15, 2026

Uh oh!

alejandro-colomar commented Jul 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

hallyn commented Jul 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

alejandro-colomar commented Dec 5, 2025 •

edited

Loading

alejandro-colomar commented Feb 26, 2026 •

edited

Loading

alejandro-colomar commented Jul 15, 2026 •

edited

Loading