[PATCH 0/4] Fedora 42: Static Analysis and Compiler Warning Fixes

newer
[PATCH v3] test: Display count of...

older
[PATCH v2] test: Display count of...

Laurent Vivier

13 May 2025 13 May '25

11:40 a.m.

This patch series addresses a collection of warnings and errors flagged by GCC and Clang's static analyzer across various modules. The fixes primarily involve clarifying code intent where warnings were false positives or applying specific attributes to suppress warnings for intentionally designed code constructs. The series consists of the following patches (in order of application): 1. **dhcpv6: fix GCC error (unterminated-string-initialization)** Silences GCC error for an intentionally non-NUL-terminated string. 2. **virtio: Fix Clang warning (bugprone-sizeof-expression, cert-arr39-c)** Justifies intentional `sizeof` usage in pointer arithmetic against Clang warnings. 3. **ndp: Fix Clang analyzer warning (clang-analyzer-security.PointerSub)** Clarifies pointer subtraction as a valid C idiom for struct offset calculation. 4. **flow: fix clang error (clang-analyzer-security.PointerSub)** Confirms pointers in `flow_idx()` reference the same array, validating subtraction. Laurent Vivier (4): dhcpv6: fix GCC error (unterminated-string-initialization) virtio: Fix Clang warning (bugprone-sizeof-expression, cert-arr39-c) ndp: Fix Clang analyzer warning (clang-analyzer-security.PointerSub) flow: Fix clang error (clang-analyzer-security.PointerSub) dhcpv6.c | 4 +++- flow_table.h | 1 + ndp.c | 1 + virtio.c | 1 + 4 files changed, 6 insertions(+), 1 deletion(-) -- 2.49.0

Show replies by date

Laurent Vivier

13 May 13 May

11:40 a.m.

New subject: [PATCH 1/4] dhcpv6: fix GCC error (unterminated-string-initialization)

The string STR_NOTONLINK is intentionally not NUL-terminated. Ignore the GCC error using __attribute__((nonstring)). This error is reported by GCC 15.1.1 on Fedora 42. However, Clang 20.1.3 does not support __attribute__((nonstring)). Therefore, NOLINTNEXTLINE(clang-diagnostic-unknown-attributes) is also added to suppress Clang's unknown attribute warning. Signed-off-by: Laurent Vivier --- dhcpv6.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/dhcpv6.c b/dhcpv6.c index 373a98869f9b..ba16c664ee24 100644 --- a/dhcpv6.c +++ b/dhcpv6.c @@ -144,7 +144,9 @@ struct opt_ia_addr { struct opt_status_code { struct opt_hdr hdr; uint16_t code; - char status_msg[sizeof(STR_NOTONLINK) - 1]; + /* "nonstring" is only supported since clang 23 */ + /* NOLINTNEXTLINE(clang-diagnostic-unknown-attributes) */ + __attribute__((nonstring)) char status_msg[sizeof(STR_NOTONLINK) - 1]; } __attribute__((packed)); /** -- 2.49.0

Laurent Vivier

11:41 a.m.

New subject: [PATCH 2/4] virtio: Fix Clang warning (bugprone-sizeof-expression, cert-arr39-c)

In `virtqueue_read_indirect_desc()`, the pointer arithmetic involving `desc` is intentional. We add the length in bytes (`read_len`) divided by the size of `struct vring_desc` to `desc`, which is an array of `struct vring_desc`. This correctly calculates the offset in terms of the number of `struct vring_desc` elements. Clang issues the following warning due to this explicit scaling: virtio.c:238:8: error: suspicious usage of 'sizeof(...)' in pointer arithmetic; this scaled value will be scaled again by the '+=' operator [bugprone-sizeof-expression,cert-arr39-c,-Werror] 238 | desc += read_len / sizeof(struct vring_desc); | ^ ~~~~~~~~~~~~~~~~~~~~~~~~~ virtio.c:238:8: note: '+=' in pointer arithmetic internally scales with 'sizeof(struct vring_desc)' == 16 This behavior is intended, so the warning can be considered a false positive in this context. The code correctly advances the pointer by the desired number of descriptor entries. Signed-off-by: Laurent Vivier --- virtio.c | 1 + 1 file changed, 1 insertion(+) diff --git a/virtio.c b/virtio.c index bc2b89a7f4a7..f7db0073b66c 100644 --- a/virtio.c +++ b/virtio.c @@ -235,6 +235,7 @@ static int virtqueue_read_indirect_desc(const struct vu_dev *dev, memcpy(desc, orig_desc, read_len); len -= read_len; addr += read_len; + /* NOLINTNEXTLINE(bugprone-sizeof-expression,cert-arr39-c) */ desc += read_len / sizeof(struct vring_desc); } -- 2.49.0

Laurent Vivier

11:41 a.m.

New subject: [PATCH 3/4] ndp: Fix Clang analyzer warning (clang-analyzer-security.PointerSub)

Addresses Clang warning: "Subtraction of two pointers that do not point into the same array is undefined behavior" for the line: `ndp_send(c, dst, &ra, ptr - (unsigned char *)&ra);` Here, `ptr` is `&ra.var[0]`. The subtraction calculates the offset of `var[0]` within the `struct ra_options ra`. Since `ptr` points inside `ra`, this pointer arithmetic is well-defined for calculating the size of the data to send, even if `ptr` and `&ra` are not strictly considered part of the same "array" by the analyzer. Signed-off-by: Laurent Vivier --- ndp.c | 1 + 1 file changed, 1 insertion(+) diff --git a/ndp.c b/ndp.c index ded2081ddd1d..b664034b00b1 100644 --- a/ndp.c +++ b/ndp.c @@ -328,6 +328,7 @@ static void ndp_ra(const struct ctx *c, const struct in6_addr *dst) memcpy(&ra.source_ll.mac, c->our_tap_mac, ETH_ALEN); + /* NOLINTNEXTLINE(clang-analyzer-security.PointerSub) */ ndp_send(c, dst, &ra, ptr - (unsigned char *)&ra); } -- 2.49.0

Laurent Vivier

11:41 a.m.

New subject: [PATCH 4/4] flow: Fix clang error (clang-analyzer-security.PointerSub)

Fixes the following clang-analyzer warning: flow_table.h:96:25: note: Subtraction of two pointers that do not point into the same array is undefined behavior 96 | return (union flow *)f - flowtab; The `flow_idx()` function is called via `FLOW_IDX()` from `flow_foreach_slot()`, where `f` is set to `&flowtab[idx].f`. Therefore, `f` and `flowtab` do point to the same array. Signed-off-by: Laurent Vivier --- flow_table.h | 1 + 1 file changed, 1 insertion(+) diff --git a/flow_table.h b/flow_table.h index 2d5c65ca4d94..3f3f4b7527bf 100644 --- a/flow_table.h +++ b/flow_table.h @@ -93,6 +93,7 @@ extern union flow flowtab[]; */ static inline unsigned flow_idx(const struct flow_common *f) { + /* NOLINTNEXTLINE(clang-analyzer-security.PointerSub) */ return (union flow *)f - flowtab; } -- 2.49.0

Stefano Brivio

14 May 14 May

6:27 p.m.

On Tue, 13 May 2025 11:40:58 +0200 Laurent Vivier wrote:

...

This patch series addresses a collection of warnings and errors flagged by GCC and Clang's static analyzer across various modules. The fixes primarily involve clarifying code intent where warnings were false positives or applying specific attributes to suppress warnings for intentionally designed code constructs.

The series consists of the following patches (in order of application):

1. **dhcpv6: fix GCC error (unterminated-string-initialization)** Silences GCC error for an intentionally non-NUL-terminated string.

2. **virtio: Fix Clang warning (bugprone-sizeof-expression, cert-arr39-c)** Justifies intentional `sizeof` usage in pointer arithmetic against Clang warnings.

3. **ndp: Fix Clang analyzer warning (clang-analyzer-security.PointerSub)** Clarifies pointer subtraction as a valid C idiom for struct offset calculation.

4. **flow: fix clang error (clang-analyzer-security.PointerSub)** Confirms pointers in `flow_idx()` reference the same array, validating subtraction.

Laurent Vivier (4): dhcpv6: fix GCC error (unterminated-string-initialization) virtio: Fix Clang warning (bugprone-sizeof-expression, cert-arr39-c) ndp: Fix Clang analyzer warning (clang-analyzer-security.PointerSub) flow: Fix clang error (clang-analyzer-security.PointerSub)

Applied. I also checked this against musl (with Clang) on Alpine, no new errors from 'make clang-tidy'. -- Stefano

Age (days ago)

Last active (days ago)

List overview

Download

5 comments

2 participants

participants (2)

Laurent Vivier
Stefano Brivio