0
extern void myprint(unsigned char *);

static inline void
myfunc(unsigned char *buf)
{
    for (unsigned int i = 0; i < 20; i++) {
        buf[i] = i;
    }
}

int
main(void)
{
    unsigned char buf[10];
    myfunc(buf);
    myprint(buf);

    return 0;
}
$ gcc.exe -O2 -pedantic -Wextra -Wall -Wstringop-overflow -Wuninitialized -Wunused -Warray-bounds=2 -Wformat-overflow -Wstringop-overread -g  -c C:\temp\cb\main.c -o build\mingw\obj\main.o
$ gcc.exe  -o build\mingw\test.exe build\mingw\obj\main.o build\mingw\obj\mod.o  -O2
$ gcc --version
gcc (i686-posix-sjlj-rev1, Built by MinGW-W64 project) 11.2.0
Copyright (C) 2021 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

If the inline function is written directly inside main(), gcc complains with an array out-of-bounds warning, but why it isn't able to detect the same warning with a static inline function?

I don't know very well the output assembler, but it seems to me the loop generated really is for 20 times, so the overbound is real during execution.

Edited at 1st December 2023, 09:13

After reading some replies, I tried to change myfunc() API, but with -O2, no warning is emitted by gcc.

#include <stddef.h>

extern void myprint(unsigned char *);

static inline void
myfunc(unsigned char *buf, size_t size)
{
    for (unsigned int i = 0; i < size + 1; i++) {
        buf[i] = i;
    }
}

int
main(void)
{
    unsigned char buf[10];
    myfunc(buf, 10);
    myprint(buf);

    return 0;
}

2 Answers 2

5

inline is just a (mostly obsolete) recommendation to the compiler, it doesn't enforce inlining. As it happens, you do get the warning under -O3 but not under -O2. Some things of note:

  • Either -O2/-O3 option does inline the function (on x86_64 gcc 13.3) but in different ways and one way with -O3 happens to be one that renders the diagnostic message.
  • In this case, the presence/absence inline keyword doesn't change the generated assembly nor the decision to inline the slightest.
  • __attribute__((always_inline)) has no affect on whether you get the warning or not either.

The best practice here is not to rely on gcc giving this warning - out of bounds access bugs in C rarely ever comes with the luxury of compiler warnings. Instead you could design the function in a type safe way:

static inline void myfunc(unsigned char buf[20])

or

static inline void myfunc(size_t size, unsigned char buf[size])

Or if taken to the extreme (very safe but cumbersome API):

static inline void myfunc(unsigned char (*buf)[20])
Sign up to request clarification or add additional context in comments.

4 Comments

I added inline keyword just to help the compiler detect the array out-of-bounds access. In my opinion, it should be able to detect the issue even without inline keyword. What I learned is that gcc isn't so good to detect these kind of issues at compile time.
As you can see in my edited question, I tried to pass the size of the array to myfunc(), but event in this case gcc doesn't detect the issue.
@pozzugno "In my opinion, it should be able to detect the issue even without inline keyword" This isn't obvious, because if not inlined/static then the compiler has to give the function external linkage and assume that the function may be called by other translation units. And then without it getting inlined into the same translation unit as where it was declared, the compiler has no way to assume what that function does internally.
but myfunc() IS static
0

If you compile your program with the option -fsanitize=address you will get array bounds checking at runtime. However, gcc on Windows does not support this option but clang does. See this answer for installation instructions: How to install MSYS2 clang with address and UB sanitizers

Here is what you will get with the sanitizer:

$ cat test.c
extern void myprint(unsigned char *);

static inline void
myfunc(unsigned char *buf)
{
    for (unsigned int i = 0; i < 20; i++) {
        buf[i] = i;
    }
}

int
main(void)
{
    unsigned char buf[10];
    myfunc(buf);
    /*myprint(buf);*/

    return 0;
}
$ clang -o test -fsanitize=address -g -Wall test.c
$ ./test.exe
=================================================================
==14280==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x007bafcff7ca at pc 0x7ff7144315
72 bp 0x007bafcff6c0 sp 0x007bafcff708
WRITE of size 1 at 0x007bafcff7ca thread T0
    #0 0x7ff714431571 in myfunc C:/msys64/home/august.karlstrom/test/test.c:7:16
    #1 0x7ff71443148b in main C:/msys64/home/august.karlstrom/test/test.c:15:5
    #2 0x7ff714431314 in __tmainCRTStartup C:/M/B/src/mingw-w64/mingw-w64-crt/crt/crtexe.c:267:15
    #3 0x7ff714431365 in .l_start C:/M/B/src/mingw-w64/mingw-w64-crt/crt/crtexe.c:188:9
    #4 0x7fff5d0e7343  (C:\Windows\System32\KERNEL32.DLL+0x180017343)
    #5 0x7fff5d6c26b0  (C:\Windows\SYSTEM32\ntdll.dll+0x1800526b0)

Address 0x007bafcff7ca is located in stack of thread T0 at offset 42 in frame
    #0 0x7ff7144313af in main C:/msys64/home/august.karlstrom/test/test.c:13

  This frame has 1 object(s):
    [32, 42) 'buf' (line 14) <== Memory access at offset 42 overflows this variable
HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcont
ext or vfork
      (longjmp, SEH and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-buffer-overflow C:/msys64/home/august.karlstrom/test/test.c:7:16 in
myfunc
Shadow bytes around the buggy address:
  0x007bafcff500: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x007bafcff580: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x007bafcff600: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x007bafcff680: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x007bafcff700: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x007bafcff780: 00 00 00 00 f1 f1 f1 f1 00[02]f3 f3 00 00 00 00
  0x007bafcff800: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x007bafcff880: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x007bafcff900: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x007bafcff980: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x007bafcffa00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==14280==ABORTING

This is the interesting line in the output:

    #0 0x7ff714431571 in myfunc C:/msys64/home/august.karlstrom/test/test.c:7:16

For completeness sake, the program should of course be written something like this:

#define LEN(array) (sizeof (array) / sizeof (array)[0])

extern void myprint(unsigned char *);

static void myfunc(unsigned char *buf, int bufLen)
{
    for (unsigned int i = 0; i < bufLen; i++) {
        buf[i] = i;
    }
}

int main(void)
{
    unsigned char buf[10];
    myfunc(buf, LEN(buf));
    myprint(buf);

    return 0;
}

2 Comments

sizeof (array)[0] will work but is quite cryptic. Why do you have a parenthesis here? The operator precedence is parenthesis over [] over sizeof. Notably, the () are not part of the sizeof operator in this example. It would be clearer to write sizeof (array) / sizeof (array[0])
@Lundin It's common best practice to enclose all macro arguments in parentheses to avoid unexpected order of evaluation.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.