[clang][ThreadSafety] Check trylock function success and return types #95290

dmcardle · 2024-06-12T19:08:41Z

With this change, Clang will generate errors when trylock functions have improper return types. Today, it silently fails to apply the trylock attribute to these functions which may incorrectly lead users to believe they have correctly acquired locks before accessing guarded data.

As a side effect of explicitly checking the success argument type, I seem to have fixed a false negative in the analysis that could occur when a trylock's success argument is an enumerator. I've added a regression test to warn-thread-safety-analysis.cpp named TrylockSuccessEnumFalseNegative.

This change also improves the documentation with descriptions of of the subtle gotchas that arise from the analysis interpreting the success arg as a boolean.

Issue #92408

github-actions · 2024-06-12T19:08:57Z

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be
notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write
permissions for the repository. In which case you can instead tag reviewers by
name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review
by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate
is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

llvmbot · 2024-06-12T19:21:59Z

@llvm/pr-subscribers-clang-analysis

@llvm/pr-subscribers-clang

Author: Dan McArdle (dmcardle)

Changes

With this change, Clang will generate a warning when a constructor is annotated as a trylock function.

Issue #92408

Full diff: https://github.com/llvm/llvm-project/pull/95290.diff

2 Files Affected:

(modified) clang/lib/Sema/SemaDeclAttr.cpp (+6)
(modified) clang/test/SemaCXX/warn-thread-safety-parsing.cpp (+7)

diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index ce6b5b1ff6f93..373f6a591fd09 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -605,6 +605,12 @@ static void handleAllocSizeAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
 
 static bool checkTryLockFunAttrCommon(Sema &S, Decl *D, const ParsedAttr &AL,
                                       SmallVectorImpl<Expr *> &Args) {
+  if (isa<CXXConstructorDecl>(D)) {
+    S.Diag(AL.getLoc(), diag::warn_attribute_wrong_decl_type)
+        << AL << AL.isRegularKeywordAttribute() << ExpectedFunction;
+    return false;
+  }
+
   if (!AL.checkAtLeastNumArgs(S, 1))
     return false;
 
diff --git a/clang/test/SemaCXX/warn-thread-safety-parsing.cpp b/clang/test/SemaCXX/warn-thread-safety-parsing.cpp
index 1626e8375892a..c9a2ad498ff24 100644
--- a/clang/test/SemaCXX/warn-thread-safety-parsing.cpp
+++ b/clang/test/SemaCXX/warn-thread-safety-parsing.cpp
@@ -747,6 +747,13 @@ class EtfFoo {
     // expected-warning {{'exclusive_trylock_function' attribute without capability arguments refers to 'this', but 'EtfFoo' isn't annotated with 'capability' or 'scoped_lockable' attribute}}
 };
 
+// It does not make sense to annotate a constructor as an exclusive trylock
+// function. See <https://github.com/llvm/llvm-project/issues/92408>.
+class EtfConstructor {
+  EtfConstructor() EXCLUSIVE_TRYLOCK_FUNCTION(1); // \
+  // expected-warning {{'exclusive_trylock_function' attribute only applies to functions}}
+};
+
 class EXCLUSIVE_TRYLOCK_FUNCTION(1) EtfTestClass { // \
   // expected-warning {{'exclusive_trylock_function' attribute only applies to functions}}
 };

dmcardle · 2024-06-12T20:15:04Z

@AaronBallman, please take a look!

What do you think of this approach? I suppose it might be even better to enforce that the function's return type is bool, which would align with the ThreadSafetyAnalysis documentation.

AaronBallman

I can see the logic, but it seems like there's a fair amount of related checking that perhaps should happen as well. Marking a destructor as try_lock makes even less sense than marking a constructor, so why not prohibit that as well? And since the try_lock attributes specify what a successful return value is, perhaps we should warn when the function declaration returns something other than int or bool. WDYT?

That said, I think one possible reason why it makes sense to allow a constructor to be try_lock is exceptions -- the ctor could lock on normal flow control and remain unlocked on exceptional flow control. However, I'm not certain we implement the analysis in an exception-aware way.

Also, the documentation (clang/docs/ThreadSafetyAnalysis.rst) should be updated and you should add a release note (clang/docs/ReleaseNotes.rst) so users know about the change.

AaronBallman · 2024-06-17T13:47:12Z

@AaronBallman, please take a look!

What do you think of this approach? I suppose it might be even better to enforce that the function's return type is bool, which would align with the ThreadSafetyAnalysis documentation.

Oops, I missed this comment -- it seems we had similar lines of thought. :-)

dmcardle · 2024-06-17T20:59:32Z

Thanks, @AaronBallman!

I tried strictly enforcing a bool return type, but I ran into some interesting test failures.

A handful of functions in the parsing tests return void and yet are annotated as trylock functions. I don't believe this is a valid use case. What does EXCLUSIVE_TRYLOCK_FUNCTION mean when it's impossible to return a value? I think the solution is to just change void to bool for trylock functions in tests.

llvm-project/clang/test/SemaCXX/warn-thread-safety-parsing.cpp

Line 725 in 06aa078

void etf_function_args() EXCLUSIVE_TRYLOCK_FUNCTION(1, mu2);
This next failure is tricker... I was really surprised to see a trylock method that returns a pointer! Confusingly, the "success" parameter to the attribute is 1. I think the semantics work out so that returning a non-nullptr value indicates successful mutex acquisition.

I'm not quite sure what to do here. Maybe we should be lenient and only enforce that the return type is boolean, integer, or pointer?

llvm-project/clang/test/SemaCXX/warn-thread-safety-analysis.cpp

Line 3661 in 06aa078

Mutex* tryLockMutexP() EXCLUSIVE_TRYLOCK_FUNCTION(1, mu_);

dmcardle · 2024-06-17T23:51:29Z

Just force-pushed what I described in the last comment.

Changed void-returning trylock functions to return bool in tests. (Probably needs another pass to minimize the diff and undo some unnecessary changes.)
Now enforcing bool/int/pointer returns from trylock-annotated functions.
Also updated the Thread Safety Analysis doc a bit.

PTAL. Thanks!

clang/test/SemaCXX/warn-thread-safety-analysis.cpp

AaronBallman · 2024-06-18T12:43:36Z

A handful of functions in the parsing tests return void and yet are annotated as trylock functions. I don't believe this is a valid use case. What does EXCLUSIVE_TRYLOCK_FUNCTION mean when it's impossible to return a value? I think the solution is to just change void to bool for trylock functions in tests.

Agreed

This next failure is tricker... I was really surprised to see a trylock method that returns a pointer! Confusingly, the "success" parameter to the attribute is 1. I think the semantics work out so that returning a non-nullptr value indicates successful mutex acquisition.

Oof, that's an odd one, but I can see the logic behind supporting it. I think it would be good for us to call out support for this explicitly in the documentation.

I'm not quite sure what to do here. Maybe we should be lenient and only enforce that the return type is boolean, integer, or pointer?

We definitely need to support integer return types because of how often integers or enumerations are used to indicate errors. I think we should continue to support the pointer behavior given that it works already today.

clang/lib/Sema/SemaDeclAttr.cpp

AaronBallman · 2024-06-18T12:40:15Z

clang/lib/Sema/SemaDeclAttr.cpp

+  if (!ReturnType->isBooleanType() && !ReturnType->isIntegerType() &&
+      !ReturnType->isPointerType()) {
+    S.Diag(AL.getLoc(), diag::err_attribute_wrong_decl_type)
+        << AL << AL.isRegularKeywordAttribute()
+        << ExpectedFunctionReturningBoolIntegerOrPointer;
+    return false;
+  }
+
  return true;
 }


Should we flip the logic here so we're looking for:

if (ReturnType->isBooleanType() || ReturnType->isIntegerType()...) return true; return !S.Diag(...);

I'm adding a some new logic for checking the success argument type. If I leave this as is, they will have the same style. If you feel strongly, I'm happy to apply DeMorgan's law here :)

AaronBallman

LGTM -- that new documentation is fantastic, thank you for that!

dmcardle · 2024-06-21T15:58:25Z

LGTM -- that new documentation is fantastic, thank you for that!

Thank you! I hope it saves future readers some time.

I don't have write access, so feel free to merge unless we're waiting for @aaronpuchert to review :)

AaronBallman · 2024-06-21T16:26:11Z

LGTM -- that new documentation is fantastic, thank you for that!

Thank you! I hope it saves future readers some time.

I don't have write access, so feel free to merge unless we're waiting for @aaronpuchert to review :)

I'll give Aaron a chance to weigh in and land the changes on your behalf early next week if we don't hear back from him.

dmcardle · 2024-06-21T20:33:02Z

So... I appear to have accidentally fixed a bug. Looks like enum success values were not being evaluated, and defaulted to false. As a result, the analysis could fail to detect unguarded access [demo on godbolt.org]. I think that the old isIntOrBool() function accidentally let enumerator values through, but the analysis assumed it would only ever see CXXBoolLiteralExpr or IntegerLiteral. I'm adding a regression test that closely resembles the demo, but verifies that the analysis actually detects the unguarded access.

I also realized I was supposed to update the release notes.

New patch with these changes coming momentarily.

With this change, Clang will generate errors when trylock functions have improper return types. Today, it silently fails to apply the trylock attribute to these functions which may incorrectly lead users to believe they have correctly acquired locks before accessing guarded data. As a side effect of explicitly checking the success argument type, I seem to have fixed a false negative in the analysis that could occur when a trylock's success argument is an enumerator. I've added a regression test to warn-thread-safety-analysis.cpp named `TrylockSuccessEnumFalseNegative`. This change also improves the documentation with descriptions of of the subtle gotchas that arise from the analysis interpreting the success arg as a boolean. Issue llvm#92408

AaronBallman

LGTM still

github-actions · 2024-06-24T13:29:34Z

@dmcardle Congratulations on having your first Pull Request (PR) merged into the LLVM Project!

Your changes will be combined with recent changes from other authors, then tested
by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR.

Please check whether problems have been caused by your change specifically, as
the builds can include changes from many authors. It is not uncommon for your
change to be included in a build that fails due to someone else's changes, or
infrastructure issues.

How to do this, and the rest of the post-merge process, is covered in detail here.

If your change does cause a problem, it may be reverted, or you can revert it yourself.
This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again.

If you don't get any reports, no action is required from you. Your changes are working as expected, well done!

asmok-g · 2024-06-27T11:36:03Z

Hi!

Is it expected for this to fail with unique_ptrs ? We have a case in the code where the return type is an alias to unique_ptr and it's failing. Also, cases where the return type is something like

absl::Nullable<std::unique_ptr<const Obj, ObjUnlocker>>

that is failing. Is that expected ?

Update:
Important reference: https://github.com/abseil/abseil-cpp/blob/master/absl/base/thread_annotations.h#L232

dmcardle · 2024-06-27T13:34:44Z

Hi @asmok-g, it was definitely not my intention to break any use cases where the analysis was behaving properly!

I wanted to verify that the prior release, which predates this change, understands smart pointer returns on trylock functions, but I'm having trouble finding a test case where the analysis behaves that way. In this example on godbolt.org with Clang 18.1.0, I mocked up (1) a plain unique_ptr return value and (2) a nullable unique_ptr type alias. It looks like the analysis ~~incorrectly~~ undesirably emits an error for unguarded access in both scenarios.

Could you help me construct an example of something that used to work and is now broken? Regardless, it still might make sense to add support for any return type that can be implicitly converted to bool.

asmok-g · 2024-06-28T10:38:50Z

Sorry for late reply. Sending in minutes!

asmok-g · 2024-06-28T10:54:39Z

https://godbolt.org/z/s6nTh6xxd does this serve ?

sorry if it's a bad code.. but it serves the purpose I think.

aaronpuchert · 2024-06-30T22:46:01Z

Thanks for looping me in and sorry for the late reply!

However, I'm not certain we implement the analysis in an exception-aware way.

If I remember correctly, the CFG is still without exception-handling edges by default, and support for those edges is still rudimentary and breaks other warnings that operate on the same CFG. It would definitely be nice, but it might be a bigger project.

I was really surprised to see a trylock method that returns a pointer! Confusingly, the "success" parameter to the attribute is 1. I think the semantics work out so that returning a non-nullptr value indicates successful mutex acquisition.

We use something like this in our code base, though I believe we've written it as true. Basically, the function returns some kind of handle/ticket that needs to be passed into the unlock method.

Maybe we should be lenient and only enforce that the return type is boolean, integer, or pointer?

We can probably generalize this a bit. I think we should be checking for contextually convertibility to bool. This would cover bool, pointers, smart pointers, and other custom types with an explicit operator bool(). Support for arbitrary integers/enumerations would also be nice, but it's a bit harder to track the data flow for them. We can probably only support them unchanged, whereas for booleans we currently allow negations and some comparisons. Didn't check how well this is supported.

aaronpuchert

Don't want to give the wrong impression, I like the idea of this change a lot.

But I think we need to be clearer about the attribute, the return value, and the relation between them. The attribute needs to be a constant expression for sure, and it's probably Ok to limit it to literals. I don't see a reason to do any kind of computation in there, or have something dependent on other values in the code. This would seem to make for an awful interface. (But I'm happy to be proven wrong with compelling use cases.)

The actual return value of the function need not be of the same type. Pointers have been mentioned, smart pointers as well. Handles could also be interesting. Anything that converts to bool, or more generally, the type in the attribute, should be fine. The idea being, as long as I can write

auto ret = mu.tryLock();
if (ret)
   /*...*/;

we should support it. This doesn't fit the nullptr case, but we could mildly generalize that. Or we ask users to write false instead, see also my comment below.

aaronpuchert · 2024-06-30T22:51:06Z

clang/lib/Analysis/ThreadSafety.cpp

-  // Find out which branch has the lock
-  bool branch = false;
-  if (const auto *BLE = dyn_cast_or_null<CXXBoolLiteralExpr>(BrE))
-    branch = BLE->getValue();
-  else if (const auto *ILE = dyn_cast_or_null<IntegerLiteral>(BrE))
-    branch = ILE->getValue().getBoolValue();


I kind of prefer the old code here. Regardless of what we accept as return type, we only care about the return type converted to bool, so other values don't really make sense as of now. (We need the IntegerLiteral for C.)

What we should document is that this isn't an equality comparison, but a comparison after conversion to bool. (Or whatever the equivalent for C is. Basically what is allowed in an if condition.)

Should we support integer/enumeration values properly, that would of course change the picture. In that case we can allow enumerators in the attribute, and the return type must be explicitly (?) convertible to type of the attribute.

Thanks for the feedback, @aaronpuchert! I've reverted the change and I'll work through these comments for a reland.

My instinct is to explicitly disallow enumerator success expressions. Currently, the analysis can produce both false negatives and false positives when enumerator success expressions are used: https://godbolt.org/z/MPYdK6hYb. To my knowledge, enumerators were never actually supported in success expressions, so in some sense this will only break code that was already broken. However, I do want to be sensitive to downstream breakage based on my experience with this PR!

One suggestion was to add a flag that disables any new errors, the goal being to enabling large code bases to incrementally adapt to the new behavior. I'm just not sure how this flag would actually work.

If we suppress any new type errors, but otherwise apply the attribute, the analysis is now off the rails, operating on unexpected inputs.

If we simply ignore any attributes that would cause an error to be emitted, now the analysis can produce different kinds of incorrect results, e.g. it might complain that you're unlocking a lock that was never acquired.

I suppose we could mimic the current/incorrect behavior when the flag is present. Is it normally expected to keep incorrect behavior around for compatibility?

@asmok-g, do you have any thoughts? How are breaking changes such as this typically handled?

My instinct is to explicitly disallow enumerator success expressions. [...] To my knowledge, enumerators were never actually supported in success expressions, so in some sense this will only break code that was already broken.

Agreed.

Is it normally expected to keep incorrect behavior around for compatibility?

It depends. If the incorrect behavior is being relied upon, or the correction could produce hard-to-find issues like different behavior at runtime, then it could make sense to keep it. But here I find it unlikely that someone relies on this, as it does not properly work, and the additional error would at most make compilation fail and would be trivial to fix.

How are breaking changes such as this typically handled?

I'd argue that the change isn't breaking anything, it's merely diagnosing a case that was already not appropriately handled.

aaronpuchert · 2024-06-30T22:55:58Z

clang/test/SemaCXX/warn-thread-safety-analysis.cpp

+
+
+// This test demonstrates that the analysis does not detect when all enumerators
+// are positive, and thus incapable of representing a failure.


positive → non-zero or nonzero.

aaronpuchert · 2024-06-30T23:00:22Z

clang/test/SemaCXX/warn-thread-safety-parsing.cpp

+#ifdef CPP11
+enum class TrylockSuccessEnumClass { NotAcquired = 0, Acquired};
+int etf_succ_enum_class_ret_i()
+    EXCLUSIVE_TRYLOCK_FUNCTION(TrylockSuccessEnumClass::Acquired, mu2);
+bool etf_succ_enum_class_ret_b()
+    EXCLUSIVE_TRYLOCK_FUNCTION(TrylockSuccessEnumClass::Acquired, mu2);
+TrylockResult *etf_succ_enum_class_ret_p()
+    EXCLUSIVE_TRYLOCK_FUNCTION(TrylockSuccessEnumClass::Acquired, mu2);
+#endif


I think enum class should not be allowed for now, because we can't convert it to bool.

Should we add proper support for enumerations, we should probably not convert them to bool but rather check for equality.

aaronpuchert · 2024-06-30T23:14:51Z

clang/lib/Sema/SemaDeclAttr.cpp

-  if (!isIntOrBool(AL.getArgAsExpr(0))) {
+  // The attribute's first argument defines the success value.
+  const Expr *SuccessArg = AL.getArgAsExpr(0);
+  if (!isa<CXXNullPtrLiteralExpr>(SuccessArg) &&


I can see the appeal of nullptr in the attribute, but it suggests you can put arbitrary pointers in there, and I don't think we want that. But I can live with it.

Though it seems strange that one would return a pointer, and nullptr is success. Maybe an error message? More typical is probably to have nullptr if the lock couldn't be acquired.

In any event, since we want to able to express both the null and non-null case, conversion to bool seems the better conceptual framework. How much we need bother users with that framework is another question, but it's probably not a bad idea to bring home the idea that we're not trying to track pointers.

…rn types (llvm#95290)" This reverts commit c1bde0a.

…97293) This PR reverts #95290 and the one-liner followup PR #96494. I received some substantial feedback on #95290, which I plan to address in a future PR. I've also received feedback that because the change emits errors where they were not emitted before, we should at least have a flag to disable the stricter warnings.

…lvm#97293) This PR reverts llvm#95290 and the one-liner followup PR llvm#96494. I received some substantial feedback on llvm#95290, which I plan to address in a future PR. I've also received feedback that because the change emits errors where they were not emitted before, we should at least have a flag to disable the stricter warnings.

…llvm#95290) With this change, Clang will generate errors when trylock functions have improper return types. Today, it silently fails to apply the trylock attribute to these functions which may incorrectly lead users to believe they have correctly acquired locks before accessing guarded data. As a side effect of explicitly checking the success argument type, I seem to have fixed a false negative in the analysis that could occur when a trylock's success argument is an enumerator. I've added a regression test to warn-thread-safety-analysis.cpp named `TrylockSuccessEnumFalseNegative`. This change also improves the documentation with descriptions of of the subtle gotchas that arise from the analysis interpreting the success arg as a boolean. Issue llvm#92408

dmcardle force-pushed the dmcardle-thread-safety-warning-constructor branch from f717082 to 9a9bf80 Compare June 12, 2024 19:20

dmcardle marked this pull request as ready for review June 12, 2024 19:21

llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Jun 12, 2024

shafik requested a review from AaronBallman June 13, 2024 19:56

AaronBallman requested a review from aaronpuchert June 17, 2024 13:37

AaronBallman reviewed Jun 17, 2024

View reviewed changes

dmcardle force-pushed the dmcardle-thread-safety-warning-constructor branch from 9a9bf80 to e53ddc9 Compare June 17, 2024 23:48

llvmbot added the clang:analysis label Jun 17, 2024

dmcardle commented Jun 17, 2024

View reviewed changes

clang/test/SemaCXX/warn-thread-safety-analysis.cpp Outdated Show resolved Hide resolved

AaronBallman reviewed Jun 18, 2024

View reviewed changes

dmcardle force-pushed the dmcardle-thread-safety-warning-constructor branch from e53ddc9 to dc8e7b1 Compare June 20, 2024 21:55

dmcardle changed the title ~~[clang][ThreadSafety] Warn when constructor is trylock~~ [clang][ThreadSafety] Check trylock function success and return types Jun 20, 2024

AaronBallman approved these changes Jun 21, 2024

View reviewed changes

dmcardle force-pushed the dmcardle-thread-safety-warning-constructor branch from dc8e7b1 to 66715ea Compare June 21, 2024 20:35

dmcardle force-pushed the dmcardle-thread-safety-warning-constructor branch from 66715ea to 807953f Compare June 21, 2024 21:23

AaronBallman approved these changes Jun 24, 2024

View reviewed changes

AaronBallman merged commit c1bde0a into llvm:main Jun 24, 2024
6 checks passed

This was referenced Jun 24, 2024

[clang][ThreadSafety] Fix code block syntax in ThreadSafetyAnalysis.rst #96494

Merged

Clarify support for exclusive_trylock_function annotation on constructors #92408

Open

aaronpuchert reviewed Jun 30, 2024

View reviewed changes

dmcardle added a commit to dmcardle/llvm-project that referenced this pull request Jul 1, 2024

Revert "[clang][ThreadSafety] Check trylock function success and retu…

783f56b

…rn types (llvm#95290)" This reverts commit c1bde0a.

dmcardle mentioned this pull request Jul 1, 2024

[clang][ThreadSafety] Revert stricter typing on trylock attributes #97293

Merged

dmcardle mentioned this pull request Jul 1, 2024

[clang][ThreadSafety] Improve errors for trylock functions #97333

Open

6 tasks



		// This test demonstrates that the analysis does not detect when all enumerators
		// are positive, and thus incapable of representing a failure.

[clang][ThreadSafety] Check trylock function success and return types #95290

[clang][ThreadSafety] Check trylock function success and return types #95290

Uh oh!

Conversation

dmcardle commented Jun 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jun 12, 2024

Uh oh!

llvmbot commented Jun 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dmcardle commented Jun 12, 2024

Uh oh!

AaronBallman left a comment

Choose a reason for hiding this comment

Uh oh!

AaronBallman commented Jun 17, 2024

Uh oh!

dmcardle commented Jun 17, 2024

Uh oh!

dmcardle commented Jun 17, 2024

Uh oh!

Uh oh!

AaronBallman commented Jun 18, 2024

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AaronBallman left a comment

Choose a reason for hiding this comment

Uh oh!

dmcardle commented Jun 21, 2024

Uh oh!

AaronBallman commented Jun 21, 2024

Uh oh!

dmcardle commented Jun 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AaronBallman left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented Jun 24, 2024

Uh oh!

asmok-g commented Jun 27, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dmcardle commented Jun 27, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

asmok-g commented Jun 28, 2024

Uh oh!

asmok-g commented Jun 28, 2024

Uh oh!

aaronpuchert commented Jun 30, 2024

Uh oh!

aaronpuchert left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dmcardle commented Jun 12, 2024 •

edited

Loading

llvmbot commented Jun 12, 2024 •

edited

Loading

dmcardle commented Jun 21, 2024 •

edited

Loading

asmok-g commented Jun 27, 2024 •

edited

Loading

dmcardle commented Jun 27, 2024 •

edited

Loading