================
@@ -1490,6 +1490,49 @@ void
Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD,
<< FunctionName << DestinationStr << SourceStr);
}
+void Sema::checkFortifiedLibcArgument(FunctionDecl *FD, CallExpr *TheCall) {
+ if (TheCall->isValueDependent() || TheCall->isTypeDependent() ||
+ isConstantEvaluatedContext())
+ return;
+ if (!FD->isExternC())
+ return;
+ const IdentifierInfo *II = FD->getIdentifier();
+ if (!II)
+ return;
+
+ // umask(mode_t): warn when the constant-evaluated argument has bits set
+ // outside the file-permission mask (0777). Those bits are ignored.
+ // Require a matching system-header declaration to avoid warning on
+ // user-defined lookalikes.
+ auto AnyDeclInSystemHeader = [&](const FunctionDecl *F) {
----------------
nuclearcat wrote:
> Hmmm so the builtin logic was not accounting for functions named `read` or
> `write` with internal linkage and treating them as builtins? Or did these
> have external linkage (at which point the user has UB because the POSIX
> library they link against also has external linkage symbols with the same
> names)? Or something else?
Good question, I went and checked rather than trusting my (bad, due stress)
memory, and it's narrower than "treated as builtins". It wasn't internal
linkage, and it wasn't codegen or UB. It was purely a front-end diagnostic:
-Wincompatible-library-redeclaration (warn_redecl_library_builtin).
Mechanism: once read/write are in the builtin table, clang materializes an
implicit prototype for them (ssize_t read(int, void *, size_t)). When someone
has their own external-linkage, file-scope read/write in C with a different
signature (say int read(char *)), it gets merged against that implicit
prototype, conflicts, and warns. The function isn't actually treated as a
builtin or miscompiled - clang only attaches builtin-ness when the signature
matches; the mismatched ones just get the warning. And static ones are excluded
entirely (I checked, static int vfork(int); gives no library-redeclaration
warning, just -Wundefined-internal).
So why did it hurt for read/write specifically? Those names are everywhere,
lots of real C code declares its own read/write with different signatures, so a
pile of previously-clean code started warning. That's what got them pulled and
dispatched by name instead.
Now,for umask: I checked whether IgnoreSignature dodges this, and it doesn't,
vfork is IgnoreSignature and a mismatched int vfork(int); still warns. So
making umask a builtin does reintroduce the same class of warning in principle.
The difference is impact, umask is a rare, warning is suppressed in system
headers, so the libc's own <sys/stat.h> won't warn even on targets where mode_t
is 16-bit and wouldn't match whatever width we encode. Only a hand-written,
non-system umask with a mismatched signature would trip it.
So it lands right where you suggested, and the "extra mile" if it ever bites
would be giving the prototype a target-resolved mode_t (the way pid_t goes
through getProcessIDType), which doesn't exist in TargetInfo today, so it'd be
a small net-new addition.
I'll get the prototype wired up and report back.
https://github.com/llvm/llvm-project/pull/198130
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
