MaggieYi created this revision.
MaggieYi added reviewers: Whitney, jamieschmeiser, MaskRay, rnk, aras-p,
anton-afanasyev.
Herald added a subscriber: hiraditya.
Herald added a project: All.
MaggieYi requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.
`HandleEndOfFile` is invoked when the lexer hits the end of the current file.
This either returns the EOF token or pops a level off the include stack and
keeps going. If it keeps going, clang parses from one header to another header.
This results in incorrect header hierarchy in the time trace.
This patch corrects this, as reported by #56554
Fixes: https://github.com/llvm/llvm-project/issues/56554
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D147520
Files:
clang/lib/Sema/Sema.cpp
clang/test/Driver/Inputs/header5.h
clang/test/Driver/Inputs/header6.h
clang/test/Driver/check-time-trace-header-hierarchy.cpp
clang/test/Driver/check-time-trace-header-hierarchy.py
llvm/include/llvm/Support/TimeProfiler.h
llvm/lib/Support/TimeProfiler.cpp
Index: llvm/lib/Support/TimeProfiler.cpp
===================================================================
--- llvm/lib/Support/TimeProfiler.cpp
+++ llvm/lib/Support/TimeProfiler.cpp
@@ -107,7 +107,7 @@
Detail());
}
- void end() {
+ void element_end() {
assert(!Stack.empty() && "Must call begin() first");
TimeTraceProfilerEntry &E = Stack.back();
E.End = ClockType::now();
@@ -143,6 +143,33 @@
Stack.pop_back();
}
+ void end(bool IsSource = false) {
+ bool TopElementIsSource = Stack.back().Name == "Source";
+ // If time trace profiler is ended by exiting a file (IsSource=true), we
+ // expect the top of Stack is time section entry with name "Source".
+ // If IsSource is false, the last time section should not be "Source". In
+ // these two cases, only pop the top element of the Stack.
+ if ((IsSource && TopElementIsSource) ||
+ (!IsSource && !TopElementIsSource)) {
+ element_end();
+ return;
+ }
+
+ // If the last time section entry is "Source" but time trace profiler is not
+ // ended by exiting a file (IsSource=false), we don't need to do anything
+ // since the time section entry has been popped out when exiting the
+ // previous 'Source' time section entry.
+ if (!IsSource && TopElementIsSource)
+ return;
+
+ // If the last time section entry is not "Source" but time trace profiler is
+ // ended by exiting a file (IsSource=true), we will pop time sections from
+ // the top of Stack until the 'Source' time section.
+ while (Stack.back().Name != "Source")
+ element_end();
+ element_end();
+ }
+
// Write events from this TimeTraceProfilerInstance and
// ThreadTimeTraceProfilerInstances.
void write(raw_pwrite_stream &OS) {
@@ -353,7 +380,7 @@
TimeTraceProfilerInstance->begin(std::string(Name), Detail);
}
-void llvm::timeTraceProfilerEnd() {
+void llvm::timeTraceProfilerEnd(bool IsSource) {
if (TimeTraceProfilerInstance != nullptr)
- TimeTraceProfilerInstance->end();
+ TimeTraceProfilerInstance->end(IsSource);
}
Index: llvm/include/llvm/Support/TimeProfiler.h
===================================================================
--- llvm/include/llvm/Support/TimeProfiler.h
+++ llvm/include/llvm/Support/TimeProfiler.h
@@ -124,8 +124,10 @@
void timeTraceProfilerBegin(StringRef Name,
llvm::function_ref<std::string()> Detail);
-/// Manually end the last time section.
-void timeTraceProfilerEnd();
+/// Manually end the last time section, with the given \p IsSource.
+/// Pass true to \p IsSource if the name of the last time section is
+/// "Source". By default, \p IsSource is false.
+void timeTraceProfilerEnd(bool IsSource = false);
/// The TimeTraceScope is a helper class to call the begin and end functions
/// of the time trace profiler. When the object is constructed, it begins
Index: clang/test/Driver/check-time-trace-header-hierarchy.py
===================================================================
--- /dev/null
+++ clang/test/Driver/check-time-trace-header-hierarchy.py
@@ -0,0 +1,50 @@
+#!/usr/bin/env python
+
+import json, sys, time
+
+def is_inside(range1, range2):
+ a = range1["ts"]; b = a + range1["dur"]
+ c = range2["ts"]; d = c + range2["dur"]
+ return (c <= a <= d) and (c <= b <= d)
+
+def is_before(range1, range2):
+ b = range1["ts"] + range1["dur"]; c = range2["ts"]
+ return b <= c
+
+log_contents = json.loads(sys.stdin.read())
+events = log_contents["traceEvents"]
+
+count = 0;
+header5 = header6 = parsetemplate = parseclass = None
+for event in events:
+ if event["name"] == "Source" and "header5.h" in event["args"]["detail"]:
+ header5 = event
+ count += 1
+ elif event["name"] == "Source" and "header6.h" in event["args"]["detail"]:
+ header6 = event
+ count += 1
+ elif event["name"] == "ParseTemplate" and event["args"]["detail"] == "Zero":
+ parsetemplate = event
+ count += 1
+ elif event["name"] == "ParseClass" and event["args"]["detail"] == "Bla":
+ parseclass = event
+ count += 1
+ elif count == 4:
+ break
+
+beginning_of_time = log_contents["beginningOfTime"] / 1000000
+seconds_since_epoch = time.time()
+
+# Make sure that the 'beginningOfTime' is not later than now.
+if beginning_of_time > seconds_since_epoch:
+ sys.exit("'beginningOfTime' should represent the absolute time when the "
+ "process has started")
+
+if not is_inside(parsetemplate, header5):
+ sys.exit("ParseTemplate `Zero` is NOT inside header5!")
+
+if not is_inside(parseclass, header6):
+ sys.exit("ParseClass `Bla` is NOT inside header6!")
+
+if not is_before(header5, header6):
+ sys.exit("header5 is NOT before header6!")
Index: clang/test/Driver/check-time-trace-header-hierarchy.cpp
===================================================================
--- /dev/null
+++ clang/test/Driver/check-time-trace-header-hierarchy.cpp
@@ -0,0 +1,6 @@
+// RUN: %clangxx -S -I %S/Inputs -ftime-trace=- -ftime-trace-granularity=0 -o /dev/null %s \
+// RUN: | %python %S/check-time-trace-header-hierarchy.py
+
+#include "header5.h"
+#include "header6.h"
+int foo();
Index: clang/test/Driver/Inputs/header6.h
===================================================================
--- /dev/null
+++ clang/test/Driver/Inputs/header6.h
@@ -0,0 +1 @@
+struct Bla {};
Index: clang/test/Driver/Inputs/header5.h
===================================================================
--- /dev/null
+++ clang/test/Driver/Inputs/header5.h
@@ -0,0 +1 @@
+template <typename T> auto Zero() -> T { return T{}; }
Index: clang/lib/Sema/Sema.cpp
===================================================================
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -166,7 +166,7 @@
case ExitFile:
if (!IncludeStack.empty()) {
if (llvm::timeTraceProfilerEnabled())
- llvm::timeTraceProfilerEnd();
+ llvm::timeTraceProfilerEnd(true);
S->DiagnoseNonDefaultPragmaAlignPack(
Sema::PragmaAlignPackDiagnoseKind::ChangedStateAtExit,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
