Valgrind (spurious?) warning/error on exec

penguin_quest · 07-31-2023, 12:52 PM

Hi, I'm not sure if I'm using the exec* syscalls correctly. Valgrind gives me an error for this minimal example:

Code:

#include <unistd.h>
int main() { execl("/bin/w", NULL); }

Here is the error:

Code:

==11213== Memcheck, a memory error detector
==11213== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al.
==11213== Using Valgrind-3.21.0 and LibVEX; rerun with -h for copyright info
==11213== Command: ./test
==11213== 
==11213== Syscall param execve(argv[0]) points to unaddressable byte(s)
==11213==    at 0x496CB47: execve (syscall-template.S:120)
==11213==    by 0x496CFB5: execl (execl.c:56)
==11213==    by 0x109155: main (in /home/glt/git/wm/test)
==11213==  Address 0x0 is not stack'd, malloc'd or (recently) free'd
==11213==

However, this semantically different variant gives no error:

Code:

#include <unistd.h>
int main() { execl("/bin/w", "", NULL); }

I thought that the first use above is correct, since the execl argument list is supposed to end with NULL. Is there another equivalent way that won't generate the Valgrind error? (It's annoying to have the spurious errors mixed in with actual program errors.) Thanks!

astrogeek · 07-31-2023, 02:12 PM

Quote:

Originally Posted by penguin_quest

I thought that the first use above is correct, since the execl argument list is supposed to end with NULL.

It is not correct.

From man 3 exec:

Code:

       int execl(const char *pathname, const char *arg, ...
                       /*, (char *) NULL */);
       ...


DESCRIPTION
       ...
       The  const char *arg and subsequent ellipses can be thought of as arg0, arg1, ..., argn.
       Together they describe a list of one or more pointers to null-terminated strings that 
       represent  the  argument  list  available to the executed program.
       ...  
       The list of arguments must be terminated by a null pointer, and, since these are variadic
       functions, this pointer must be cast (char *) NULL.

NevemTeve · 07-31-2023, 03:04 PM

Code:

-    int main() { execl("/bin/w", NULL); }
+    int main() { execl("/bin/w", "/bin/w", NULL); }
C++  int main() { execl("/bin/w", "/bin/w", (char *)0); }

(C++ renders symbol NULL unusable, thank you Bjarne Strupstrup)

GazL · 07-31-2023, 06:38 PM

Quote:

... The list of arguments must be terminated by a null pointer, and, since these are variadic functions, this pointer must be cast (char *) NULL.

Why? What's wrong with just passing NULL with no cast?

astrogeek · 07-31-2023, 07:43 PM

Quote:

Originally Posted by GazL

Why? What's wrong with just passing NULL with no cast?

Nothing at all as far as I know, perhaps I should have not hilighted that sentence but tend to be more pedantic when quoting man pages. In fact, is valgrind even aware of those casts?

But to be perhaps a bit more clear, what valgrind is complaining about is the NULL second parameter...

Code:

Syscall param execve(argv[0]) points to unaddressable byte(s)

The function prototype and man page are clear that the first and second parameters are required and must be non-null (i.e. addressable) pointers.

Code:

int execl(const char *pathname, const char *arg, ... /*, (char *) NULL */);

The  const char *arg and subsequent ellipses can be thought of as arg0, 
arg1, ..., argn.  Together they describe a list of one or more pointers 
to null-terminated strings that represent  the  argument  list  available
 to the executed program.

The important take-away here is that both *pathname and *arg are required and must be non-null. That is, the second parameter, *arg, demarks the beginning of the optional parameters and cannot itself serve as the NULL pointer terminating the optional parameter list.

Even though the compiler and runtime platform may accept a NULL second parameter, a non-null pointer is required and it is this which valgrind is complaining about. To silence that complaint simply supply a non-null pointer - as NevemTeve has somewhat obliquely pointed out, this is typically used to pass the program name, argv[0].

ntubski · 07-31-2023, 09:01 PM

Quote:

Originally Posted by GazL

Why? What's wrong with just passing NULL with no cast?

My guess is that there could be platforms where the size of a void* is different from char*.

NevemTeve · 07-31-2023, 10:52 PM

C defines NULL as (void *)0, while C++ defines NULL as 0
In variadic context the latter can cause problems.

GazL · 08-01-2023, 05:02 AM

Quote:

Originally Posted by astrogeek

Nothing at all as far as I know, perhaps I should have not hilighted that sentence but tend to be more pedantic when quoting man pages. In fact, is valgrind even aware of those casts?

But to be perhaps a bit more clear, what valgrind is complaining about is the NULL second parameter...

Yes, thanks astrogeek. I understood what the problem with the execl() was, I just didn't understand the insistence in the man-page to casting it to char *. I wondered whether I was missing something, and I think I was. I've done a little google-searching and come up with a possible rationale based on some of the posts I found on related topics:

The C standard states that the definition of NULL is implementation defined, and can either be the integral 0, or (void *) 0. In the context of pointer comparison/assignment these two are treated identically (as a null pointer). but in the case where NULL is defined as 0 and when that 0 is seen in a variadic parameter (where the compiler has no idea what type is expected at the other end), that 0 may likely be seen as int (size 4) rather than a null pointer (size 8, on 64bit).

If this is indeed the issue, then (char *) 0 is likely more correct as it matches the type expected in execl() but I don't think it matters what the pointer type is so long as it's a pointer, so (void *) 0 is likely just as good. However, NULL may or may not be defined appropriately.

Of course, C++ had the good sense to introduce 'nullptr' avoiding this sort of thing.

I tend to avoid writing variadic functions in general, and I always use execv() rather than execl(), so I hadn't really thought about this stuff in much detail before. It's possible I just learned something.