From: Akira Hatanaka Date: Tue, 6 Nov 2018 07:05:14 +0000 (+0000) Subject: os_log: Allow specifying mask type in format string. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=0d698165769071a002414537024f9fe5e1ead2f4;p=clang os_log: Allow specifying mask type in format string. A mask type is a 1 to 8-byte string that follows the "mask." annotation in the format string. This enables obfuscating data in the event the provided privacy level isn't enabled. rdar://problem/36756282 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@346211 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/AST/FormatString.h b/include/clang/AST/FormatString.h index 39eb6258a6..1e341f3840 100644 --- a/include/clang/AST/FormatString.h +++ b/include/clang/AST/FormatString.h @@ -477,6 +477,7 @@ class PrintfSpecifier : public analyze_format_string::FormatSpecifier { OptionalFlag IsPublic; // '{public}' OptionalFlag IsSensitive; // '{sensitive}' OptionalAmount Precision; + StringRef MaskType; public: PrintfSpecifier() : FormatSpecifier(/* isPrintf = */ true), HasThousandsGrouping("'"), @@ -559,6 +560,9 @@ public: const OptionalFlag &isSensitive() const { return IsSensitive; } bool usesPositionalArg() const { return UsesPositionalArg; } + StringRef getMaskType() const { return MaskType; } + void setMaskType(StringRef S) { MaskType = S; } + /// Changes the specifier and length according to a QualType, retaining any /// flags or options. Returns true on success, or false when a conversion /// was not successful. @@ -691,6 +695,9 @@ public: return true; } + /// Handle mask types whose sizes are not between one and eight bytes. + virtual void handleInvalidMaskType(StringRef MaskType) {} + // Scanf-specific handlers. virtual bool HandleInvalidScanfConversionSpecifier( diff --git a/include/clang/AST/OSLog.h b/include/clang/AST/OSLog.h index c8895156d0..2b21855e7a 100644 --- a/include/clang/AST/OSLog.h +++ b/include/clang/AST/OSLog.h @@ -52,7 +52,10 @@ public: // The item is corresponding to the '%m' format specifier, no value is // populated in the buffer and the runtime is loading the errno value. - ErrnoKind + ErrnoKind, + + // The item is a mask type. + MaskKind }; enum { @@ -72,10 +75,13 @@ private: CharUnits ConstValue; CharUnits Size; // size of the data, not including the header bytes unsigned Flags = 0; + StringRef MaskType; public: - OSLogBufferItem(Kind kind, const Expr *expr, CharUnits size, unsigned flags) - : TheKind(kind), TheExpr(expr), Size(size), Flags(flags) { + OSLogBufferItem(Kind kind, const Expr *expr, CharUnits size, unsigned flags, + StringRef maskType = StringRef()) + : TheKind(kind), TheExpr(expr), Size(size), Flags(flags), + MaskType(maskType) { assert(((Flags == 0) || (Flags == IsPrivate) || (Flags == IsPublic) || (Flags == IsSensitive)) && "unexpected privacy flag"); @@ -99,6 +105,8 @@ public: const Expr *getExpr() const { return TheExpr; } CharUnits getConstValue() const { return ConstValue; } CharUnits size() const { return Size; } + + StringRef getMaskType() const { return MaskType; } }; class OSLogBufferLayout { @@ -122,9 +130,10 @@ public: Items, [](const OSLogBufferItem &Item) { return Item.getIsPrivate(); }); } - bool hasNonScalar() const { + bool hasNonScalarOrMask() const { return llvm::any_of(Items, [](const OSLogBufferItem &Item) { - return Item.getKind() != OSLogBufferItem::ScalarKind; + return Item.getKind() != OSLogBufferItem::ScalarKind || + !Item.getMaskType().empty(); }); } @@ -132,7 +141,7 @@ public: unsigned char result = 0; if (hasPrivateItems()) result |= HasPrivateItems; - if (hasNonScalar()) + if (hasNonScalarOrMask()) result |= HasNonScalarItems; return result; } diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index d322457d77..dc616e72a4 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -7902,6 +7902,8 @@ def warn_format_non_standard: Warning< def warn_format_non_standard_conversion_spec: Warning< "using length modifier '%0' with conversion specifier '%1' is not supported by ISO C">, InGroup, DefaultIgnore; +def err_invalid_mask_type_size : Error< + "mask type size must be between 1-byte and 8-bytes">; def warn_format_invalid_annotation : Warning< "using '%0' format specifier annotation outside of os_log()/os_trace()">, InGroup; diff --git a/lib/AST/OSLog.cpp b/lib/AST/OSLog.cpp index d70aa60518..df2f808728 100644 --- a/lib/AST/OSLog.cpp +++ b/lib/AST/OSLog.cpp @@ -26,6 +26,7 @@ private: Optional Precision; Optional FieldWidth; unsigned char Flags = 0; + StringRef MaskType; }; SmallVector ArgsData; ArrayRef Args; @@ -127,12 +128,19 @@ public: else if (FS.isPublic()) ArgsData.back().Flags |= OSLogBufferItem::IsPublic; + ArgsData.back().MaskType = FS.getMaskType(); return true; } void computeLayout(ASTContext &Ctx, OSLogBufferLayout &Layout) const { Layout.Items.clear(); for (auto &Data : ArgsData) { + if (!Data.MaskType.empty()) { + CharUnits Size = CharUnits::fromQuantity(8); + Layout.Items.emplace_back(OSLogBufferItem::MaskKind, nullptr, + Size, 0, Data.MaskType); + } + if (Data.FieldWidth) { CharUnits Size = Ctx.getTypeSizeInChars((*Data.FieldWidth)->getType()); Layout.Items.emplace_back(OSLogBufferItem::ScalarKind, *Data.FieldWidth, diff --git a/lib/AST/PrintfFormatString.cpp b/lib/AST/PrintfFormatString.cpp index a92375b452..c57d8f3642 100644 --- a/lib/AST/PrintfFormatString.cpp +++ b/lib/AST/PrintfFormatString.cpp @@ -127,7 +127,8 @@ static PrintfSpecifierResult ParsePrintfSpecifier(FormatStringHandler &H, do { StringRef Str(I, E - I); - std::string Match = "^[[:space:]]*(private|public|sensitive)" + std::string Match = "^[[:space:]]*" + "(private|public|sensitive|mask\\.[^[:space:],}]*)" "[[:space:]]*(,|})"; llvm::Regex R(Match); SmallVector Matches; @@ -139,7 +140,13 @@ static PrintfSpecifierResult ParsePrintfSpecifier(FormatStringHandler &H, // Set the privacy flag if the privacy annotation in the // comma-delimited segment is at least as strict as the privacy // annotations in previous comma-delimited segments. - if (MatchedStr.equals("sensitive")) + if (MatchedStr.startswith("mask")) { + StringRef MaskType = MatchedStr.substr(sizeof("mask.") - 1); + unsigned Size = MaskType.size(); + if (Warn && (Size == 0 || Size > 8)) + H.handleInvalidMaskType(MaskType); + FS.setMaskType(MaskType); + } else if (MatchedStr.equals("sensitive")) PrivacyFlags = clang::analyze_os_log::OSLogBufferItem::IsSensitive; else if (PrivacyFlags != clang::analyze_os_log::OSLogBufferItem::IsSensitive && diff --git a/lib/CodeGen/CGBuiltin.cpp b/lib/CodeGen/CGBuiltin.cpp index 76b46c7a5f..4699a0c16a 100644 --- a/lib/CodeGen/CGBuiltin.cpp +++ b/lib/CodeGen/CGBuiltin.cpp @@ -1168,7 +1168,12 @@ RValue CodeGenFunction::emitBuiltinOSLogFormat(const CallExpr &E) { llvm::Value *ArgVal; - if (const Expr *TheExpr = Item.getExpr()) { + if (Item.getKind() == analyze_os_log::OSLogBufferItem::MaskKind) { + uint64_t Val = 0; + for (unsigned I = 0, E = Item.getMaskType().size(); I < E; ++I) + Val |= ((unsigned )Item.getMaskType()[I]) << I * 8; + ArgVal = llvm::Constant::getIntegerValue(Int64Ty, llvm::APInt(64, Val)); + } else if (const Expr *TheExpr = Item.getExpr()) { ArgVal = EmitScalarExpr(TheExpr, /*Ignore*/ false); // Check if this is a retainable type. diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp index 4660ed55ae..bdc942c142 100644 --- a/lib/Sema/SemaChecking.cpp +++ b/lib/Sema/SemaChecking.cpp @@ -7135,6 +7135,8 @@ public: const char *startSpecifier, unsigned specifierLen) override; + void handleInvalidMaskType(StringRef MaskType) override; + bool HandlePrintfSpecifier(const analyze_printf::PrintfSpecifier &FS, const char *startSpecifier, unsigned specifierLen) override; @@ -7186,6 +7188,10 @@ bool CheckPrintfHandler::HandleInvalidPrintfConversionSpecifier( CS.getStart(), CS.getLength()); } +void CheckPrintfHandler::handleInvalidMaskType(StringRef MaskType) { + S.Diag(getLocationOfByte(MaskType.data()), diag::err_invalid_mask_type_size); +} + bool CheckPrintfHandler::HandleAmount( const analyze_format_string::OptionalAmount &Amt, unsigned k, const char *startSpecifier, diff --git a/test/CodeGen/builtins.c b/test/CodeGen/builtins.c index 40d7ca32d7..2506b2d484 100644 --- a/test/CodeGen/builtins.c +++ b/test/CodeGen/builtins.c @@ -454,6 +454,22 @@ void test_builtin_os_log(void *buf, int i, const char *data) { // CHECK: call void @__os_log_helper_1_3_1_8_37( __builtin_os_log_format(buf, "%{ private, sensitive, private, public}s", "abc"); + + // CHECK: store volatile i32 22, i32* %[[LEN]], align 4 + len = __builtin_os_log_format_buffer_size("%{mask.xyz}s", "abc"); + + // CHECK: call void @__os_log_helper_1_2_2_8_112_8_34(i8* {{.*}}, i64 8026488 + __builtin_os_log_format(buf, "%{mask.xyz, public}s", "abc"); + + // CHECK: call void @__os_log_helper_1_3_2_8_112_4_1(i8* {{.*}}, i64 8026488 + __builtin_os_log_format(buf, "%{ mask.xyz, private }d", 11); + + // Mask type is silently ignored. + // CHECK: call void @__os_log_helper_1_2_1_8_32( + __builtin_os_log_format(buf, "%{ mask. xyz }s", "abc"); + + // CHECK: call void @__os_log_helper_1_2_1_8_32( + __builtin_os_log_format(buf, "%{ mask.xy z }s", "abc"); } // CHECK-LABEL: define linkonce_odr hidden void @__os_log_helper_1_3_4_4_0_8_34_4_17_8_49 diff --git a/test/SemaObjC/format-strings-oslog.m b/test/SemaObjC/format-strings-oslog.m index 15c88e1b37..e8b1d64f07 100644 --- a/test/SemaObjC/format-strings-oslog.m +++ b/test/SemaObjC/format-strings-oslog.m @@ -39,6 +39,10 @@ void test_os_log_format(const char *pc, int i, void *p, void *buf) { struct { char data[0x100]; } toobig; __builtin_os_log_format(buf, "%s", toobig); // expected-error {{os_log() argument 2 is too big (256 bytes, max 255)}} + + __builtin_os_log_format(buf, "%{mask.xyz}s", "abc"); + __builtin_os_log_format(buf, "%{mask.}s", "abc"); // expected-error {{mask type size must be between 1-byte and 8-bytes}} + __builtin_os_log_format(buf, "%{mask.abcdefghi}s", "abc"); // expected-error {{mask type size must be between 1-byte and 8-bytes}} } // Test os_log_format primitive with ObjC string literal format argument.