[PATCH] D57918: Add an attribute that causes clang to emit fortified calls to C stdlib functions

John McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Feb 7 19:25:03 PST 2019

rjmccall added inline comments.

Comment at: clang/include/clang/Basic/AttrDocs.td:986
+  }
+  }];
Probably worth clarifying somewhere in here that only a specific set of stdlib functions are supported.  I don't know that it's important to spell that set out.

Comment at: clang/lib/CodeGen/CGBuiltin.cpp:1490
+  if (auto *Attr = FD->getAttr<FortifyStdLibAttr>()) {
+    SmallVector<llvm::Value *, 8> ArgVals;
Could you move the body of this into a helper function?  This function is really far too large as it is.

Comment at: clang/lib/CodeGen/CGBuiltin.cpp:1516
+      // void *memset(void *b, int c, size_t len);
+      // void *__memset_chk(void *b, int c, size_t len, size_t);
+      ArgVals.push_back(emitObjSize());
I think this might be over-commented. :)

Could you just put smaller comments on groups of cases, like:
  // All of these take the destination object size as the final argument.
  // These take flags and the destination object size immediately before the format string.

Comment at: clang/test/Sema/fortify-std-lib.c:6
+__attribute__((fortify_stdlib(0, 0)))
+int not_anyting_special(); // expected-error {{'fortify_stdlib' attribute applied to an unknown function}}
Silly comment, but: you typo'd "anything" here.

  rC Clang



More information about the cfe-commits mailing list