mstorsjo wrote:
> cc @mstorsjo @sylvain-audi @aganea
Sorry, I don't really know anything about this area at all, so I don't think I
have anything to add here.
https://github.com/llvm/llvm-project/pull/66816
___
cfe-commits mailing list
@@ -3838,13 +3838,13 @@ void MicrosoftMangleContextImpl::mangleSEHFinallyBlock(
Mangler.mangleName(EnclosingDecl);
}
-void MicrosoftMangleContextImpl::mangleTypeName(
+void MicrosoftMangleContextImpl::mangleCanonicalTypeName(
QualType T, raw_ostream , bool
tru wrote:
cc @mstorsjo @sylvain-audi @aganea
https://github.com/llvm/llvm-project/pull/66816
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
mizvekov wrote:
Yep, it's a regression from 16. This bug makes ubsan unusable for MSVC target
in real world projects.
We could alternatively backport just the canonicalization, but not the rename,
if that makes it better.
https://github.com/llvm/llvm-project/pull/66816
tru wrote:
We guarantee abi/api stability between the patch versions, so I would say no.
Maybe if we think it's critical enough we could do a 17.1.0, but this doesn't
seem to be the case when reading the description at least. Is this a regression
from LLVM 16?
I could be convinced, but my
mizvekov wrote:
@tru Would you agree to backport this fix to 17?
Strictly speaking there is an API change here, but I think it's a bit obscure
and in any case, this change could catch misuse of it.
https://github.com/llvm/llvm-project/pull/66816
___
https://github.com/mizvekov closed
https://github.com/llvm/llvm-project/pull/66816
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/rnk approved this pull request.
Yep, looks good to me.
https://github.com/llvm/llvm-project/pull/66816
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
mizvekov wrote:
@rnk Since @efriedma-quic seems to be unavailable, any objections we move
forward and merge this, as is?
https://github.com/llvm/llvm-project/pull/66816
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
mizvekov wrote:
@efriedma-quic gentle ping :-)
https://github.com/llvm/llvm-project/pull/66816
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/mizvekov updated
https://github.com/llvm/llvm-project/pull/66816
>From f2e022964f00e348ad3ab04ade88182485f7c19d Mon Sep 17 00:00:00 2001
From: Matheus Izvekov
Date: Tue, 19 Sep 2023 22:21:25 +0200
Subject: [PATCH] -fsanitize=function: fix MSVC hashing to sugared type
mizvekov wrote:
I think this is a better direction overall, although it has the downside that
it might be more difficult to backport this to 17.
https://github.com/llvm/llvm-project/pull/66816
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
rnk wrote:
Seems reasonable to me, but I want @efriedma-quic to approve as well, if this
is a reasonable implementation of your direction.
https://github.com/llvm/llvm-project/pull/66816
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://github.com/mizvekov updated
https://github.com/llvm/llvm-project/pull/66816
>From 725a8b45144d6aaab71d282110619f5f843d527f Mon Sep 17 00:00:00 2001
From: Matheus Izvekov
Date: Tue, 19 Sep 2023 21:50:39 +0200
Subject: [PATCH 1/3] -fsanitize=function: Add a MSVC test case
---
mizvekov wrote:
Okay, makes sense, although we can only say for certain regarding in-tree users.
I have gone ahead and done that, plus I have renamed `mangleTypeName` to
`mangleCanonicalTypeName` so the name describes the behavior better, and since
this is an API break, we break it loudly.
https://github.com/mizvekov updated
https://github.com/llvm/llvm-project/pull/66816
>From 725a8b45144d6aaab71d282110619f5f843d527f Mon Sep 17 00:00:00 2001
From: Matheus Izvekov
Date: Tue, 19 Sep 2023 21:50:39 +0200
Subject: [PATCH 1/3] -fsanitize=function: Add a MSVC test case
---
rnk wrote:
It is true that the MSVC mangler doesn't generally canonicalize types, but I
think we can canonicalize in the `mangleTypeName` entry point, because it
exists to create unique type names for TBAA and CFI. You can audit the callers,
they all relate to either of those two things.
mizvekov wrote:
> It looks like the Itanium mangleTypeName() canonicalizes the type itself;
> should the MSVC mangleTypeName do the same thing?
It's done on purpose:
`MicrosoftMangle.cpp`:
```cpp
void MicrosoftCXXNameMangler::mangleType(QualType T, SourceRange Range,
efriedma-quic wrote:
It looks like the Itanium mangleTypeName() canonicalizes the type itself;
should the MSVC mangleTypeName do the same thing?
https://github.com/llvm/llvm-project/pull/66816
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
llvmbot wrote:
@llvm/pr-subscribers-clang
Changes
Hashing the sugared type instead of the canonical type meant that
a simple example like this would always fail under MSVC:
```
static auto l() {}
int main() {
auto a = l;
a();
}
```
`clang --target=x86_64-pc-windows-msvc
https://github.com/mizvekov created
https://github.com/llvm/llvm-project/pull/66816
Hashing the sugared type instead of the canonical type meant that
a simple example like this would always fail under MSVC:
```
static auto l() {}
int main() {
auto a = l;
a();
}
```
`clang
21 matches
Mail list logo
