[compiler-rt] r228869 - [UBSan] Allow UBSan location to store frames returned by symbolizer.
Alexey Samsonov
vonosmas at gmail.com
Wed Feb 11 11:45:08 PST 2015
Author: samsonov
Date: Wed Feb 11 13:45:07 2015
New Revision: 228869
URL: http://llvm.org/viewvc/llvm-project?rev=228869&view=rev
Log:
[UBSan] Allow UBSan location to store frames returned by symbolizer.
Summary:
__ubsan::getFunctionLocation() used to issue a call to symbolizer, and
convert the result (SymbolizedStack) to one of UBSan structures:
SourceLocation, ModuleLocation or MemoryLocation. This:
(1) is inefficient: we do an extra allocation/deallocation to copy data,
while we can instead can just pass SymbolizedStack around (which
contains all the necessary data).
(2) leaks memory: strings stored in SourceLocation/MemoryLocation are
never deallocated, and Filipe Cabecinhas suggests this causes crashes
of UBSan-ified programs in the wild.
Instead, let Location store a pointer to SymbolizedStack object, and
make sure it's properly deallocated when UBSan handler exits.
ModuleLocation is made obsolete by this change, and is deleted.
Test Plan: check-ubsan test suite
Reviewers: rsmith, filcab
Subscribers: llvm-commits
Differential Revision: http://reviews.llvm.org/D7548
Modified:
compiler-rt/trunk/lib/ubsan/ubsan_diag.cc
compiler-rt/trunk/lib/ubsan/ubsan_diag.h
compiler-rt/trunk/lib/ubsan/ubsan_handlers.cc
Modified: compiler-rt/trunk/lib/ubsan/ubsan_diag.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/ubsan/ubsan_diag.cc?rev=228869&r1=228868&r2=228869&view=diff
==============================================================================
--- compiler-rt/trunk/lib/ubsan/ubsan_diag.cc (original)
+++ compiler-rt/trunk/lib/ubsan/ubsan_diag.cc Wed Feb 11 13:45:07 2015
@@ -66,38 +66,9 @@ class Decorator : public SanitizerCommon
};
}
-Location __ubsan::getCallerLocation(uptr CallerPC) {
- if (!CallerPC)
- return Location();
-
- return getFunctionLocation(StackTrace::GetPreviousInstructionPc(CallerPC), 0);
-}
-
-Location __ubsan::getFunctionLocation(uptr PC, const char **FName) {
- if (!PC)
- return Location();
+SymbolizedStack *__ubsan::getSymbolizedLocation(uptr PC) {
InitIfNecessary();
-
- SymbolizedStack *Frames = Symbolizer::GetOrInit()->SymbolizePC(PC);
- const AddressInfo &Info = Frames->info;
-
- if (!Info.module) {
- Frames->ClearAll();
- return Location(PC);
- }
-
- if (FName && Info.function)
- *FName = internal_strdup(Info.function);
-
- if (!Info.file) {
- ModuleLocation MLoc(internal_strdup(Info.module), Info.module_offset);
- Frames->ClearAll();
- return MLoc;
- }
-
- SourceLocation SLoc(internal_strdup(Info.file), Info.line, Info.column);
- Frames->ClearAll();
- return SLoc;
+ return Symbolizer::GetOrInit()->SymbolizePC(PC);
}
Diag &Diag::operator<<(const TypeDescriptor &V) {
@@ -141,15 +112,22 @@ static void renderLocation(Location Loc)
SLoc.getColumn(), common_flags()->strip_path_prefix);
break;
}
- case Location::LK_Module: {
- ModuleLocation MLoc = Loc.getModuleLocation();
- RenderModuleLocation(&LocBuffer, MLoc.getModuleName(), MLoc.getOffset(),
- common_flags()->strip_path_prefix);
- break;
- }
case Location::LK_Memory:
LocBuffer.append("%p", Loc.getMemoryLocation());
break;
+ case Location::LK_Symbolized: {
+ const AddressInfo &Info = Loc.getSymbolizedStack()->info;
+ if (Info.file) {
+ RenderSourceLocation(&LocBuffer, Info.file, Info.line, Info.column,
+ common_flags()->strip_path_prefix);
+ } else if (Info.module) {
+ RenderModuleLocation(&LocBuffer, Info.module, Info.module_offset,
+ common_flags()->strip_path_prefix);
+ } else {
+ LocBuffer.append("%p", Info.address);
+ }
+ break;
+ }
case Location::LK_Null:
LocBuffer.append("<unknown>");
break;
Modified: compiler-rt/trunk/lib/ubsan/ubsan_diag.h
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/ubsan/ubsan_diag.h?rev=228869&r1=228868&r2=228869&view=diff
==============================================================================
--- compiler-rt/trunk/lib/ubsan/ubsan_diag.h (original)
+++ compiler-rt/trunk/lib/ubsan/ubsan_diag.h Wed Feb 11 13:45:07 2015
@@ -16,78 +16,84 @@
#include "ubsan_value.h"
#include "sanitizer_common/sanitizer_stacktrace.h"
#include "sanitizer_common/sanitizer_suppressions.h"
+#include "sanitizer_common/sanitizer_symbolizer.h"
namespace __ubsan {
-/// \brief A location within a loaded module in the program. These are used when
-/// the location can't be resolved to a SourceLocation.
-class ModuleLocation {
- const char *ModuleName;
- uptr Offset;
+class SymbolizedStackHolder {
+ SymbolizedStack *Stack;
+
+ void clear() {
+ if (Stack)
+ Stack->ClearAll();
+ }
public:
- ModuleLocation() : ModuleName(0), Offset(0) {}
- ModuleLocation(const char *ModuleName, uptr Offset)
- : ModuleName(ModuleName), Offset(Offset) {}
- const char *getModuleName() const { return ModuleName; }
- uptr getOffset() const { return Offset; }
+ explicit SymbolizedStackHolder(SymbolizedStack *Stack = nullptr)
+ : Stack(Stack) {}
+ ~SymbolizedStackHolder() { clear(); }
+ void reset(SymbolizedStack *S) {
+ if (Stack != S)
+ clear();
+ Stack = S;
+ }
+ const SymbolizedStack *get() const { return Stack; }
};
+SymbolizedStack *getSymbolizedLocation(uptr PC);
+
+inline SymbolizedStack *getCallerLocation(uptr CallerPC) {
+ CHECK(CallerPC);
+ uptr PC = StackTrace::GetPreviousInstructionPc(CallerPC);
+ return getSymbolizedLocation(PC);
+}
+
/// A location of some data within the program's address space.
typedef uptr MemoryLocation;
/// \brief Location at which a diagnostic can be emitted. Either a
-/// SourceLocation, a ModuleLocation, or a MemoryLocation.
+/// SourceLocation, a MemoryLocation, or a SymbolizedStack.
class Location {
public:
- enum LocationKind { LK_Null, LK_Source, LK_Module, LK_Memory };
+ enum LocationKind { LK_Null, LK_Source, LK_Memory, LK_Symbolized };
private:
LocationKind Kind;
// FIXME: In C++11, wrap these in an anonymous union.
SourceLocation SourceLoc;
- ModuleLocation ModuleLoc;
MemoryLocation MemoryLoc;
+ const SymbolizedStack *SymbolizedLoc; // Not owned.
public:
Location() : Kind(LK_Null) {}
Location(SourceLocation Loc) :
Kind(LK_Source), SourceLoc(Loc) {}
- Location(ModuleLocation Loc) :
- Kind(LK_Module), ModuleLoc(Loc) {}
Location(MemoryLocation Loc) :
Kind(LK_Memory), MemoryLoc(Loc) {}
+ // SymbolizedStackHolder must outlive Location object.
+ Location(const SymbolizedStackHolder &Stack) :
+ Kind(LK_Symbolized), SymbolizedLoc(Stack.get()) {}
LocationKind getKind() const { return Kind; }
bool isSourceLocation() const { return Kind == LK_Source; }
- bool isModuleLocation() const { return Kind == LK_Module; }
bool isMemoryLocation() const { return Kind == LK_Memory; }
+ bool isSymbolizedStack() const { return Kind == LK_Symbolized; }
SourceLocation getSourceLocation() const {
CHECK(isSourceLocation());
return SourceLoc;
}
- ModuleLocation getModuleLocation() const {
- CHECK(isModuleLocation());
- return ModuleLoc;
- }
MemoryLocation getMemoryLocation() const {
CHECK(isMemoryLocation());
return MemoryLoc;
}
+ const SymbolizedStack *getSymbolizedStack() const {
+ CHECK(isSymbolizedStack());
+ return SymbolizedLoc;
+ }
};
-/// Try to obtain a location for the caller. This might fail, and produce either
-/// an invalid location or a module location for the caller.
-Location getCallerLocation(uptr CallerPC);
-
-/// Try to obtain a location for the given function pointer. This might fail,
-/// and produce either an invalid location or a module location for the caller.
-/// If FName is non-null and the name of the function is known, set *FName to
-/// the function name, otherwise *FName is unchanged.
-Location getFunctionLocation(uptr PC, const char **FName);
-
/// A diagnostic severity level.
enum DiagLevel {
DL_Error, ///< An error.
Modified: compiler-rt/trunk/lib/ubsan/ubsan_handlers.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/ubsan/ubsan_handlers.cc?rev=228869&r1=228868&r2=228869&view=diff
==============================================================================
--- compiler-rt/trunk/lib/ubsan/ubsan_handlers.cc (original)
+++ compiler-rt/trunk/lib/ubsan/ubsan_handlers.cc Wed Feb 11 13:45:07 2015
@@ -43,8 +43,11 @@ static void handleTypeMismatchImpl(TypeM
if (ignoreReport(Loc.getSourceLocation(), Opts))
return;
- if (Data->Loc.isInvalid())
- Loc = getCallerLocation(Opts.pc);
+ SymbolizedStackHolder FallbackLoc;
+ if (Data->Loc.isInvalid()) {
+ FallbackLoc.reset(getCallerLocation(Opts.pc));
+ Loc = FallbackLoc;
+ }
ScopedReport R(Opts, Loc);
@@ -288,7 +291,8 @@ void __ubsan::__ubsan_handle_vla_bound_n
static void handleFloatCastOverflow(FloatCastOverflowData *Data,
ValueHandle From, ReportOptions Opts) {
// TODO: Add deduplication once a SourceLocation is generated for this check.
- Location Loc = getCallerLocation(Opts.pc);
+ SymbolizedStackHolder CallerLoc(getCallerLocation(Opts.pc));
+ Location Loc = CallerLoc;
ScopedReport R(Opts, Loc);
Diag(Loc, DL_Error,
@@ -343,8 +347,10 @@ static void handleFunctionTypeMismatch(F
ScopedReport R(Opts, CallLoc);
- const char *FName = "(unknown)";
- Location FLoc = getFunctionLocation(Function, &FName);
+ SymbolizedStackHolder FLoc(getSymbolizedLocation(Function));
+ const char *FName = FLoc.get()->info.function;
+ if (!FName)
+ FName = "(unknown)";
Diag(CallLoc, DL_Error,
"call to function %0 through pointer to incorrect function type %1")
More information about the llvm-commits
mailing list