Skip to content

Always initialize c_options=NULL - #2751

Closed
MichaelChirico wants to merge 2 commits into
igraph:mainfrom
MichaelChirico:patch-1
Closed

Always initialize c_options=NULL#2751
MichaelChirico wants to merge 2 commits into
igraph:mainfrom
MichaelChirico:patch-1

Conversation

@MichaelChirico

Copy link
Copy Markdown
Contributor

This was flagged by clang's -Wsometimes-uninitialized.

@krlmlr

krlmlr commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Thanks for raising this. Could the problem be not the c_options but that Rz_SEXP_to_igraph_arpack_options() doesn't fully initialize the whole struct?

Do we need a specific version of clang to replicate this?

@MichaelChirico

MichaelChirico commented Jul 26, 2026

Copy link
Copy Markdown
Contributor Author

This fails to compile for me:

// reprex.c
#include <stddef.h>

extern void consume_pointer(void* ptr);
extern int is_null(void* arg);

void test_pagerank_options(void* options, int algo) {
    void* c_options; // Declared without initialization

    if (!is_null(options)) {
        if (algo == 1) {
            c_options = options;
        } else {
            c_options = NULL;
        }
    }

    // When is_null(options) is true, c_options is read uninitialized here:
    consume_pointer(c_options);
}
clang -Wsometimes-uninitialized -c reprex.c

output:

reprex.c:9:9: warning: variable 'c_options' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
    if (!is_null(options)) {
        ^~~~~~~~~~~~~~~~~
reprex.c:18:21: note: uninitialized use occurs here
    consume_pointer(c_options);
                    ^~~~~~~~~
reprex.c:9:5: note: remove the 'if' if its condition is always true
    if (!is_null(options)) {
    ^~~~~~~~~~~~~~~~~~~~~~~
reprex.c:7:20: note: initialize the variable 'c_options' to silence this warning
    void* c_options; // Declared without initialization
                   ^
                    = NULL
1 warning generated.
clang --version
# Ubuntu clang version 14.0.0-1ubuntu1.1
# Target: x86_64-pc-linux-gnu
# Thread model: posix
# InstalledDir: /usr/bin

@MichaelChirico

Copy link
Copy Markdown
Contributor Author

It does have the feel of a false positive to it, since it doesn't cause any "real" issues, but there is still UB here as summarized by Gemini:

"Algorithmatically, it acts like a false positive because PRPACK ignores the options argument entirely—so it won't cause runtime errors or crashes. However, for Clang and C static analyzers, it is a true positive: in C, loading an uninitialized variable to pass it as a function argument at the call site is an uninitialized read, regardless of whether the callee dereferences it later. Initializing to NULL satisfies C evaluation rules and prevents compiler/sanitizer traps on strict build toolchains."

@krlmlr

krlmlr commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Of course, thanks for the simple example. Will adapt the code generator.

@krlmlr

krlmlr commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Thanks, included in #2792.

@krlmlr krlmlr closed this Jul 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants