Re: [PATCH] tools api io: Return high-bit bytes as unsigned characters

[Ian Rogers]

On Thu, Nov 12, 2026 at 12:25 PM szpuszi <szpuszi@xxxxxxxxx> wrote:
>
> Hi Ian,
>
> `tools/lib/api/Makefile` does not set -funsigned-char, so building
> libapi standalone (`make -C tools/lib/api` on x86_64, where char is
> signed by default) hits this directly: `filename__read_str()` calls
> `io__getdelim()` -> `io__get_char()` and silently stops on bytes in
> 0x80-0xff.
>
> Even if perf's -funsigned-char is also passed down to the libapi build,
> `io__get_char()` is a `static inline` in a public header
> (tools/lib/api/io.h) with fgetc()-style negative error/EOF returns.
> Casting to `(unsigned char)` fixes the underlying bug in the header
> itself rather than relying on every caller of <api/io.h> compiling
> with -funsigned-char. The `-fsigned-char` flag on `api-io.o` is just so
> the regression test keeps exercising this path under perf's default
> -funsigned-char build.

It is only a bug if chars are signed. I thought by default it was
unspecified whether they were signed or not, hence char having 3 types
- signed, unsigned and unspecified. At least in the Chris Mason review
prompts we're pretty explicit that chars are unsigned:
https://github.com/masoncl/review-prompts/blob/main/kernel/subsystem/build.md
```
Unsigned char (-funsigned-char): Top-level Makefile enforces unsigned
char on all architectures. Checking char < 0 is dead code or incorrect
logic.
```

> If you'd also like me to check whether perf passes -funsigned-char
> through to the libapi build and send a separate patch for that, let me
> know.

So I ran a check and I don't see `-funsigned-char` being passed into
libapi in the cc command line. So maybe there's a bug. I see
Makefile.perf exporting CFLAGS.

Thanks,
Ian

> Thanks,
> Wojciech
>
> On Thu, Nov 12, 2026 at 6:27 PM Ian Rogers <irogers@xxxxxxxxxx> wrote:
> >
> > On Thu, Nov 12, 2026 at 9:06 AM Wojciech Szpucha <szpuszi@xxxxxxxxx> wrote:
> > >
> > > From: Wojciech Szpucha <szpuszi@xxxxxxxxx>
> > >
> > > On signed-char builds, io__get_char() returns negative values for bytes
> > > 0x80-0xff. This makes filename__read_str() silently truncate non-ASCII
> > > input in standalone libapi builds.
> > >
> > > Cast the byte to unsigned char before returning it as int. Add regression
> > > tests compiled with -fsigned-char, since perf defaults to -funsigned-char
> > > and already avoids the issue.
> > >
> > > Fixes: e95770af4c4a ("tools api: Add a lightweight buffered reading api")
> > > Signed-off-by: Wojciech Szpucha <szpuszi@xxxxxxxxx>
> >
> > Can you provide more context of where this is a problem? The code
> > builds with -funsigned-char :
> > https://lore.kernel.org/linux-perf-users/20260306191908.2065682-1-irogers@xxxxxxxxxx/
> > and so we may be failing to pass CFLAGS through properly for the libapi build.
> >
> > Thanks,
> > Ian
> >
> > > ---
> > > Tested on Ubuntu 24.04 x86_64, GCC 13.3.0: standalone libapi reads 0/4
> > > bytes before the fix and 4/4 afterwards. The API IO regression fails
> > > before and passes after. libapi and perf build successfully; checkpatch
> > > passes. Full perf suite and kernel builds were not run.
> > >
> > > https://github.com/szpuszi/linux-perf-io-test/actions/runs/43257707229
> > >
> > >  tools/lib/api/io.h        |  2 +-
> > >  tools/perf/tests/Build    |  1 +
> > >  tools/perf/tests/api-io.c | 33 ++++++++++++++++++++++++++++++++-
> > >  3 files changed, 34 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/tools/lib/api/io.h b/tools/lib/api/io.h
> > > index 1731996b..25e2008d 100644
> > > --- a/tools/lib/api/io.h
> > > +++ b/tools/lib/api/io.h
> > > @@ -91,7 +91,7 @@ static inline int io__get_char(struct io *io)
> > >                 if (ret)
> > >                         return ret;
> > >         }
> > > -       return *io->data++;
> > > +       return (unsigned char)*io->data++;
> > >  }
> > >
> > >  /* Read a hexadecimal value with no 0x prefix into the out argument hex. If the
> > > diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build
> > > index 58858f3d..c3ba1fca 100644
> > > --- a/tools/perf/tests/Build
> > > +++ b/tools/perf/tests/Build
> > > @@ -79,6 +79,7 @@ perf-test-$(CONFIG_DWARF_UNWIND) += dwarf-unwind.o
> > >  endif
> > >
> > >  CFLAGS_dwarf-unwind.o += -fno-optimize-sibling-calls
> > > +CFLAGS_api-io.o += -fsigned-char
> > >
> > >  perf-test-y += workloads/
> > >
> > > diff --git a/tools/perf/tests/api-io.c b/tools/perf/tests/api-io.c
> > > index 0ba3d5cc..f283712e 100644
> > > --- a/tools/perf/tests/api-io.c
> > > +++ b/tools/perf/tests/api-io.c
> > > @@ -98,7 +98,7 @@ static int do_test_get_char(const char *test_string,
> > > size_t buf_size)
> > >         for (i = 0; i < strlen(test_string); i++) {
> > >                 ch = io__get_char(&io);
> > >
> > > -               EXPECT_EQUAL(ch, test_string[i]);
> > > +               EXPECT_EQUAL(ch, (unsigned char)test_string[i]);
> > >                 EXPECT_EQUAL(io.eof, false);
> > >         }
> > >         ch = io__get_char(&io);
> > > @@ -118,6 +118,7 @@ static int test_get_char(void)
> > >                 "12345678abcdef90",
> > >                 "a\nb\nc\nd\n",
> > >                 "\a\b\t\v\f\r",
> > > +               "\x7f\x80\xfe\xff",
> > >         };
> > >         for (i = 0; i <= 10; i++) {
> > >                 for (j = 0; j < ARRAY_SIZE(test_strings); j++) {
> > > @@ -290,6 +291,34 @@ static int test_get_dec(void)
> > >         return ret;
> > >  }
> > >
> > > +static int test_get_line_non_ascii(void)
> > > +{
> > > +       static const char test_string[] = "a\xc3\xa9\xff\nnext\n";
> > > +       char path[PATH_MAX];
> > > +       struct io io;
> > > +       char *line = NULL;
> > > +       size_t line_len = 0;
> > > +       int len, ret = 0;
> > > +
> > > +       if (setup_test(path, test_string, 2, &io))
> > > +               return -1;
> > > +
> > > +       len = io__getline(&io, &line, &line_len);
> > > +       EXPECT_EQUAL(len, 5);
> > > +       EXPECT_EQUAL((int)line_len, 5);
> > > +       EXPECT_EQUAL((int)strlen(line), 5);
> > > +       if (line_len == 5)
> > > +               EXPECT_EQUAL(memcmp(line, test_string, 5), 0);
> > > +       len = io__getline(&io, &line, &line_len);
> > > +       EXPECT_EQUAL(len, 5);
> > > +       EXPECT_EQUAL((int)line_len, 5);
> > > +       EXPECT_EQUAL(strcmp(line, "next\n"), 0);
> > > +
> > > +       free(line);
> > > +       cleanup_test(path, &io);
> > > +       return ret;
> > > +}
> > > +
> > >  static int test_get_line(void)
> > >  {
> > >         char path[PATH_MAX];
> > > @@ -337,6 +366,8 @@ static int test__api_io(struct test_suite *test
> > > __maybe_unused,
> > >                 ret = TEST_FAIL;
> > >         if (test_get_line())
> > >                 ret = TEST_FAIL;
> > > +       if (test_get_line_non_ascii())
> > > +               ret = TEST_FAIL;
> > >         return ret;
> > >  }
> > >
> > >
> > > base-commit: 9d66f26fefacebfeed15de31c1123451a17ff119