[clang] [clang][Sema] Add fortify warnings for `unistd.h` (PR #161737)
https://github.com/ColinKinloch created
https://github.com/llvm/llvm-project/pull/161737
This MR implements in clang some of the overflow and over-read checks
implemented in bionics [`FORTIFY`
headers](https://cs.android.com/android/platform/superproject/main/+/main:bionic/libc/include/bits/fortify/).
In order to get the checks working with the existing overflow checker this MR
defines the following as builtin:
* `read`
* `pread`/`pread64`
* `write`
* `pwrite`/`pwrite64`
* `getcwd`
* `readlink`
* `readlinkat`
The MR also enables `ssize_t` in clangs AST and adds the `off_t`, `off64_t` and
`ssize_t` types for use in builtin signatures. It also defines a new diagnostic
`warn_fortify_destination_size_mismatch` to warn when the called function may
read beyond the bounds of the given source buffer.
It also moves the comparison between source and destination sizes into each
case of the switch block, this means that a given diagnostic can be triggered
by buffer overflows or over-reads.
### Questions
Do `chk` variants of these functions e.g. `__builtin___getcwd_chk` etc. makes
sense to implement for `fortify-source`
([LSB](https://refspecs.linuxfoundation.org/LSB_5.0.0/LSB-Core-generic/LSB-Core-generic/baselib---getcwd-chk-1.html))?
Is defining `off_t` as `unsigned long` robust enough?
Are the names of `warn_fortify_destination_size_mismatch` and
`warn_fortify_source_size_mismatch` easy enough to distinguish?
### Further Work
I plan to port more of the fortify checks from the bionic headers. I'd be
interested to hear opinions on whether these belong alongside clangs
`fortify-source` diagnostics or would be better suited elsewhere in Sema.
Specifically I plan to work on:
* Checking whether the number of `fds` matches the `nfds` in a call to `poll`
([bionic](https://cs.android.com/android/platform/superproject/main/+/main:bionic/libc/include/bits/fortify/poll.h;l=54)).
* Checking buffer sizes against `SSIZE_MAX` and `PATH_MAX` for `unistd.h`
functions and `realpath`.
* Check `realpath` isn't passed a null `path` string (maybe
https://github.com/llvm/llvm-project/pull/160988 would be useful)
([bionic](https://cs.android.com/android/platform/superproject/main/+/main:bionic/libc/include/bits/fortify/stdlib.h)).
* Validating that the `flags` and `mode` values passed to `open` and `openat`
are meaningful
([bionic](https://cs.android.com/android/platform/superproject/main/+/main:bionic/libc/include/bits/fortify/fcntl.h)).
>From 98912f07d2b6f3a1d8c79d2e38407dc1d9ba2905 Mon Sep 17 00:00:00 2001
From: Colin Kinloch
Date: Thu, 2 Oct 2025 22:01:40 +0100
Subject: [PATCH] [clang][Sema] Add fortify warnings for `unistd.h`
Define as builtin and check for overflows and over-reads in:
* `read`
* `pread`/`pread64`
* `write`
* `pwrite`/`pwrite64`
* `getcwd`
* `readlink`
* `readlinkat`
It also enables `off_t`, `off64_t` and `ssize_t` for use in builtin
signatures.
Signed-off-by: Colin Kinloch
---
clang/include/clang/Basic/Builtins.td | 65 +
.../clang/Basic/DiagnosticSemaKinds.td| 4 ++
clang/lib/AST/ASTContext.cpp | 9 ++-
clang/lib/Sema/SemaChecking.cpp | 70 ++-
clang/test/Sema/warn-fortify-source.c | 55 +++
clang/utils/TableGen/ClangBuiltinsEmitter.cpp | 3 +
6 files changed, 201 insertions(+), 5 deletions(-)
diff --git a/clang/include/clang/Basic/Builtins.td
b/clang/include/clang/Basic/Builtins.td
index 468121f7d20ab..3c3a5a82a28a6 100644
--- a/clang/include/clang/Basic/Builtins.td
+++ b/clang/include/clang/Basic/Builtins.td
@@ -3490,6 +3490,8 @@ def StrnCaseCmp : GNULibBuiltin<"strings.h"> {
let RequiresUndef = 1;
}
+// POSIX unistd.h
+
def GNU_Exit : GNULibBuiltin<"unistd.h"> {
let Spellings = ["_exit"];
let Attributes = [NoReturn];
@@ -3502,6 +3504,69 @@ def VFork : LibBuiltin<"unistd.h"> {
let Prototype = "pid_t()";
}
+def Read : LibBuiltin<"unistd.h"> {
+ let Spellings = ["read"];
+ let Attributes = [NoThrow];
+ let Prototype = "ssize_t(int, void*, size_t)";
+ let AddBuiltinPrefixedAlias = 1;
+}
+
+def PRead : LibBuiltin<"unistd.h"> {
+ let Spellings = ["pread"];
+ let Attributes = [NoThrow];
+ let Prototype = "ssize_t(int, void*, size_t, off_t)";
+ let AddBuiltinPrefixedAlias = 1;
+}
+
+def PRead64 : LibBuiltin<"unistd.h"> {
+ let Spellings = ["pread64"];
+ let Attributes = [NoThrow];
+ let Prototype = "ssize_t(int, void*, size_t, off64_t)";
+ let AddBuiltinPrefixedAlias = 1;
+}
+
+def Write : LibBuiltin<"unistd.h"> {
+ let Spellings = ["write"];
+ let Attributes = [NoThrow];
+ let Prototype = "ssize_t(int, void const*, size_t)";
+ let AddBuiltinPrefixedAlias = 1;
+}
+
+def PWrite : LibBuiltin<"unistd.h"> {
+ let Spellings = ["pwrite"];
+ let Attributes = [NoThrow];
+ let Prototype = "ssize_t(int, void const*, size_t, off_t)";
+ let AddBuiltinPrefixedAlias = 1;
+}
+
+def PWrite64 : LibBuiltin<"unistd.h"> {
+ let Spellings = ["pwrite64"];
+ le
[clang] [clang][Sema] Add fortify warnings for `unistd.h` (PR #161737)
https://github.com/ColinKinloch edited https://github.com/llvm/llvm-project/pull/161737 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Add fortify warnings for `unistd.h` (PR #161737)
https://github.com/ColinKinloch edited https://github.com/llvm/llvm-project/pull/161737 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Add fortify warnings for `unistd.h` (PR #161737)
https://github.com/ColinKinloch edited https://github.com/llvm/llvm-project/pull/161737 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Add fortify warnings for `unistd.h` (PR #161737)
https://github.com/ColinKinloch updated
https://github.com/llvm/llvm-project/pull/161737
>From 98912f07d2b6f3a1d8c79d2e38407dc1d9ba2905 Mon Sep 17 00:00:00 2001
From: Colin Kinloch
Date: Thu, 2 Oct 2025 22:01:40 +0100
Subject: [PATCH 1/7] [clang][Sema] Add fortify warnings for `unistd.h`
Define as builtin and check for overflows and over-reads in:
* `read`
* `pread`/`pread64`
* `write`
* `pwrite`/`pwrite64`
* `getcwd`
* `readlink`
* `readlinkat`
It also enables `off_t`, `off64_t` and `ssize_t` for use in builtin
signatures.
Signed-off-by: Colin Kinloch
---
clang/include/clang/Basic/Builtins.td | 65 +
.../clang/Basic/DiagnosticSemaKinds.td| 4 ++
clang/lib/AST/ASTContext.cpp | 9 ++-
clang/lib/Sema/SemaChecking.cpp | 70 ++-
clang/test/Sema/warn-fortify-source.c | 55 +++
clang/utils/TableGen/ClangBuiltinsEmitter.cpp | 3 +
6 files changed, 201 insertions(+), 5 deletions(-)
diff --git a/clang/include/clang/Basic/Builtins.td
b/clang/include/clang/Basic/Builtins.td
index 468121f7d20ab..3c3a5a82a28a6 100644
--- a/clang/include/clang/Basic/Builtins.td
+++ b/clang/include/clang/Basic/Builtins.td
@@ -3490,6 +3490,8 @@ def StrnCaseCmp : GNULibBuiltin<"strings.h"> {
let RequiresUndef = 1;
}
+// POSIX unistd.h
+
def GNU_Exit : GNULibBuiltin<"unistd.h"> {
let Spellings = ["_exit"];
let Attributes = [NoReturn];
@@ -3502,6 +3504,69 @@ def VFork : LibBuiltin<"unistd.h"> {
let Prototype = "pid_t()";
}
+def Read : LibBuiltin<"unistd.h"> {
+ let Spellings = ["read"];
+ let Attributes = [NoThrow];
+ let Prototype = "ssize_t(int, void*, size_t)";
+ let AddBuiltinPrefixedAlias = 1;
+}
+
+def PRead : LibBuiltin<"unistd.h"> {
+ let Spellings = ["pread"];
+ let Attributes = [NoThrow];
+ let Prototype = "ssize_t(int, void*, size_t, off_t)";
+ let AddBuiltinPrefixedAlias = 1;
+}
+
+def PRead64 : LibBuiltin<"unistd.h"> {
+ let Spellings = ["pread64"];
+ let Attributes = [NoThrow];
+ let Prototype = "ssize_t(int, void*, size_t, off64_t)";
+ let AddBuiltinPrefixedAlias = 1;
+}
+
+def Write : LibBuiltin<"unistd.h"> {
+ let Spellings = ["write"];
+ let Attributes = [NoThrow];
+ let Prototype = "ssize_t(int, void const*, size_t)";
+ let AddBuiltinPrefixedAlias = 1;
+}
+
+def PWrite : LibBuiltin<"unistd.h"> {
+ let Spellings = ["pwrite"];
+ let Attributes = [NoThrow];
+ let Prototype = "ssize_t(int, void const*, size_t, off_t)";
+ let AddBuiltinPrefixedAlias = 1;
+}
+
+def PWrite64 : LibBuiltin<"unistd.h"> {
+ let Spellings = ["pwrite64"];
+ let Attributes = [NoThrow];
+ let Prototype = "ssize_t(int, void const*, size_t, off64_t)";
+ let AddBuiltinPrefixedAlias = 1;
+}
+
+def GetCWD : LibBuiltin<"unistd.h"> {
+ let Spellings = ["getcwd"];
+ let Attributes = [NoThrow];
+ let Prototype = "char*(char*, size_t)";
+ let AddBuiltinPrefixedAlias = 1;
+}
+
+def ReadLink : LibBuiltin<"unistd.h"> {
+ let Spellings = ["readlink"];
+ let Attributes = [NoThrow];
+ let Prototype = "ssize_t(char const* restrict, char* restrict, size_t)";
+ let AddBuiltinPrefixedAlias = 1;
+}
+
+def ReadLinkAt : LibBuiltin<"unistd.h"> {
+ let Spellings = ["readlinkat"];
+ let Attributes = [NoThrow];
+ let Prototype = "ssize_t(int, char const* restrict, char* restrict, size_t)";
+ let AddBuiltinPrefixedAlias = 1;
+}
+
// POSIX pthread.h
def PthreadCreate : GNULibBuiltin<"pthread.h"> {
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index b157cbb0b8069..f2206ad08414e 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -953,6 +953,10 @@ def warn_fortify_source_overflow
def warn_fortify_source_size_mismatch : Warning<
"'%0' size argument is too large; destination buffer has size %1,"
" but size argument is %2">, InGroup;
+def warn_fortify_destination_size_mismatch
+: Warning<"'%0' size argument is too large; source buffer has size %2,"
+ " but size argument is %1">,
+ InGroup;
def warn_fortify_strlen_overflow: Warning<
"'%0' will always overflow; destination buffer has size %1,"
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 056bfe36b2a0a..d0432de14dbb3 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -12528,9 +12528,12 @@ static QualType DecodeTypeFromStr(const char *&Str,
const ASTContext &Context,
assert(HowLong == 0 && !Signed && !Unsigned && "Bad modifiers for 'b'!");
Type = Context.BoolTy;
break;
- case 'z': // size_t.
-assert(HowLong == 0 && !Signed && !Unsigned && "Bad modifiers for 'z'!");
-Type = Context.getSizeType();
+ case 'z': // size_t and ssize_t.
+assert(HowLong == 0 && "Bad modifiers for 'z'!");
+if (Signed)
+ Type = Context.getSignedSizeType();
+else
+ Type = Context.getSizeType();
break;
case 'w': //
[clang] [clang][Sema] Add fortify warnings for `unistd.h` (PR #161737)
https://github.com/ColinKinloch updated
https://github.com/llvm/llvm-project/pull/161737
>From 98912f07d2b6f3a1d8c79d2e38407dc1d9ba2905 Mon Sep 17 00:00:00 2001
From: Colin Kinloch
Date: Thu, 2 Oct 2025 22:01:40 +0100
Subject: [PATCH 1/3] [clang][Sema] Add fortify warnings for `unistd.h`
Define as builtin and check for overflows and over-reads in:
* `read`
* `pread`/`pread64`
* `write`
* `pwrite`/`pwrite64`
* `getcwd`
* `readlink`
* `readlinkat`
It also enables `off_t`, `off64_t` and `ssize_t` for use in builtin
signatures.
Signed-off-by: Colin Kinloch
---
clang/include/clang/Basic/Builtins.td | 65 +
.../clang/Basic/DiagnosticSemaKinds.td| 4 ++
clang/lib/AST/ASTContext.cpp | 9 ++-
clang/lib/Sema/SemaChecking.cpp | 70 ++-
clang/test/Sema/warn-fortify-source.c | 55 +++
clang/utils/TableGen/ClangBuiltinsEmitter.cpp | 3 +
6 files changed, 201 insertions(+), 5 deletions(-)
diff --git a/clang/include/clang/Basic/Builtins.td
b/clang/include/clang/Basic/Builtins.td
index 468121f7d20ab..3c3a5a82a28a6 100644
--- a/clang/include/clang/Basic/Builtins.td
+++ b/clang/include/clang/Basic/Builtins.td
@@ -3490,6 +3490,8 @@ def StrnCaseCmp : GNULibBuiltin<"strings.h"> {
let RequiresUndef = 1;
}
+// POSIX unistd.h
+
def GNU_Exit : GNULibBuiltin<"unistd.h"> {
let Spellings = ["_exit"];
let Attributes = [NoReturn];
@@ -3502,6 +3504,69 @@ def VFork : LibBuiltin<"unistd.h"> {
let Prototype = "pid_t()";
}
+def Read : LibBuiltin<"unistd.h"> {
+ let Spellings = ["read"];
+ let Attributes = [NoThrow];
+ let Prototype = "ssize_t(int, void*, size_t)";
+ let AddBuiltinPrefixedAlias = 1;
+}
+
+def PRead : LibBuiltin<"unistd.h"> {
+ let Spellings = ["pread"];
+ let Attributes = [NoThrow];
+ let Prototype = "ssize_t(int, void*, size_t, off_t)";
+ let AddBuiltinPrefixedAlias = 1;
+}
+
+def PRead64 : LibBuiltin<"unistd.h"> {
+ let Spellings = ["pread64"];
+ let Attributes = [NoThrow];
+ let Prototype = "ssize_t(int, void*, size_t, off64_t)";
+ let AddBuiltinPrefixedAlias = 1;
+}
+
+def Write : LibBuiltin<"unistd.h"> {
+ let Spellings = ["write"];
+ let Attributes = [NoThrow];
+ let Prototype = "ssize_t(int, void const*, size_t)";
+ let AddBuiltinPrefixedAlias = 1;
+}
+
+def PWrite : LibBuiltin<"unistd.h"> {
+ let Spellings = ["pwrite"];
+ let Attributes = [NoThrow];
+ let Prototype = "ssize_t(int, void const*, size_t, off_t)";
+ let AddBuiltinPrefixedAlias = 1;
+}
+
+def PWrite64 : LibBuiltin<"unistd.h"> {
+ let Spellings = ["pwrite64"];
+ let Attributes = [NoThrow];
+ let Prototype = "ssize_t(int, void const*, size_t, off64_t)";
+ let AddBuiltinPrefixedAlias = 1;
+}
+
+def GetCWD : LibBuiltin<"unistd.h"> {
+ let Spellings = ["getcwd"];
+ let Attributes = [NoThrow];
+ let Prototype = "char*(char*, size_t)";
+ let AddBuiltinPrefixedAlias = 1;
+}
+
+def ReadLink : LibBuiltin<"unistd.h"> {
+ let Spellings = ["readlink"];
+ let Attributes = [NoThrow];
+ let Prototype = "ssize_t(char const* restrict, char* restrict, size_t)";
+ let AddBuiltinPrefixedAlias = 1;
+}
+
+def ReadLinkAt : LibBuiltin<"unistd.h"> {
+ let Spellings = ["readlinkat"];
+ let Attributes = [NoThrow];
+ let Prototype = "ssize_t(int, char const* restrict, char* restrict, size_t)";
+ let AddBuiltinPrefixedAlias = 1;
+}
+
// POSIX pthread.h
def PthreadCreate : GNULibBuiltin<"pthread.h"> {
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index b157cbb0b8069..f2206ad08414e 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -953,6 +953,10 @@ def warn_fortify_source_overflow
def warn_fortify_source_size_mismatch : Warning<
"'%0' size argument is too large; destination buffer has size %1,"
" but size argument is %2">, InGroup;
+def warn_fortify_destination_size_mismatch
+: Warning<"'%0' size argument is too large; source buffer has size %2,"
+ " but size argument is %1">,
+ InGroup;
def warn_fortify_strlen_overflow: Warning<
"'%0' will always overflow; destination buffer has size %1,"
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 056bfe36b2a0a..d0432de14dbb3 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -12528,9 +12528,12 @@ static QualType DecodeTypeFromStr(const char *&Str,
const ASTContext &Context,
assert(HowLong == 0 && !Signed && !Unsigned && "Bad modifiers for 'b'!");
Type = Context.BoolTy;
break;
- case 'z': // size_t.
-assert(HowLong == 0 && !Signed && !Unsigned && "Bad modifiers for 'z'!");
-Type = Context.getSizeType();
+ case 'z': // size_t and ssize_t.
+assert(HowLong == 0 && "Bad modifiers for 'z'!");
+if (Signed)
+ Type = Context.getSignedSizeType();
+else
+ Type = Context.getSizeType();
break;
case 'w': //
[clang] [clang][Sema] Add fortify warnings for `unistd.h` (PR #161737)
@@ -12528,9 +12528,12 @@ static QualType DecodeTypeFromStr(const char *&Str, const ASTContext &Context, assert(HowLong == 0 && !Signed && !Unsigned && "Bad modifiers for 'b'!"); Type = Context.BoolTy; break; - case 'z': // size_t. -assert(HowLong == 0 && !Signed && !Unsigned && "Bad modifiers for 'z'!"); -Type = Context.getSizeType(); + case 'z': // size_t and ssize_t. +assert(HowLong == 0 && "Bad modifiers for 'z'!"); +if (Signed) ColinKinloch wrote: I don't think so. Could you give me some guidance as to what tests would be suitable for this? Or pointers to relevant existing code? https://github.com/llvm/llvm-project/pull/161737 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [StaticAnalyzer] Fix non decimal macro values in tryExpandAsInteger (PR #168632)
https://github.com/ColinKinloch created
https://github.com/llvm/llvm-project/pull/168632
Values were parsed into an unsigned APInt with just enough of a bit width to
hold the number then interpreted as signed values. This resulted in hex, octal
and binary literals from being interpreted as negative when the most
significant bit is 1.
For example the `-0b11` would have a bit width of 2, would be interpreted as
-1, then negated to become 1.
>From 925247bfe6d85fab1cdb7c4f4092fe5f47033776 Mon Sep 17 00:00:00 2001
From: Colin Kinloch
Date: Tue, 18 Nov 2025 23:28:43 +
Subject: [PATCH] [StaticAnalyzer] Fix non decimal macro values in
tryExpandAsInteger
Values were parsed into an unsigned APInt with just enough of a bit
width to hold the number then interpreted as signed values. This
resulted in hex, octal and binary literals from being interpreted as
negative when the most significant bit is 1.
For example the `-0b11` would have a bit width of 2, would be
interpreted as -1, then negated to become 1.
---
clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp | 14 ++
.../Analysis/std-c-library-functions-eof-2-rad.c | 16
2 files changed, 26 insertions(+), 4 deletions(-)
create mode 100644 clang/test/Analysis/std-c-library-functions-eof-2-rad.c
diff --git a/clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp
b/clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp
index 8b404377186e9..9cf31af37f116 100644
--- a/clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp
+++ b/clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp
@@ -142,19 +142,25 @@ std::optional tryExpandAsInteger(StringRef Macro,
const Preprocessor &PP) {
if (InvalidSpelling)
return std::nullopt;
- llvm::APInt IntValue;
+ llvm::APSInt IntValue(0, true);
constexpr unsigned AutoSenseRadix = 0;
- if (ValueStr.getAsInteger(AutoSenseRadix, IntValue))
+ if (ValueStr.getAsInteger(AutoSenseRadix,
+static_cast(IntValue)))
return std::nullopt;
// Parse an optional minus sign.
size_t Size = FilteredTokens.size();
if (Size >= 2) {
-if (FilteredTokens[Size - 2].is(tok::minus))
+if (FilteredTokens[Size - 2].is(tok::minus)) {
+ // Make sure there's space for a sign bit
+ if (IntValue.isSignBitSet())
+IntValue = IntValue.extend(IntValue.getBitWidth() + 1);
+ IntValue.setIsUnsigned(false);
IntValue = -IntValue;
+}
}
- return IntValue.getSExtValue();
+ return IntValue.getExtValue();
}
OperatorKind operationKindFromOverloadedOperator(OverloadedOperatorKind OOK,
diff --git a/clang/test/Analysis/std-c-library-functions-eof-2-rad.c
b/clang/test/Analysis/std-c-library-functions-eof-2-rad.c
new file mode 100644
index 0..6ead28b97fbf7
--- /dev/null
+++ b/clang/test/Analysis/std-c-library-functions-eof-2-rad.c
@@ -0,0 +1,16 @@
+// RUN: %clang_analyze_cc1 -std=c23
-analyzer-checker=core,unix.StdCLibraryFunctions,debug.ExprInspection -verify
-analyzer-config eagerly-assume=false %s
+
+void clang_analyzer_eval(int);
+
+typedef struct FILE FILE;
+/// Test that the static analyzer doesn't interpret the most significant bit
as the sign bit.
+// Unorthodox EOF value with a power of 2 radix
+#define EOF (-0b11)
+
+int getc(FILE *);
+void test_getc(FILE *fp) {
+ int y = getc(fp);
+ if (y < 0) {
+clang_analyzer_eval(y == EOF); // expected-warning{{TRUE}}
+ }
+}
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
