Page MenuHomePhabricator

Add support for the MS qualifiers __ptr32, __ptr64, __sptr, __uptr.

Authored by akhuang on Dec 4 2019, 4:42 PM.



This adds parsing of the qualifiers ptr32, ptr64, sptr, and uptr and
lowers them to the corresponding address space pointer for 32-bit and 64-bit pointers.
(32/64-bit pointers added in

A large part of this patch is making these pointers ignore the address space
when doing things like overloading and casting.

Diff Detail

Event Timeline

akhuang created this revision.Dec 4 2019, 4:42 PM
rnk added a comment.Dec 12 2019, 4:22 PM

Sorry for the delay, overall this seems like the right approach.


Can this be simplified to:

((isPtrSizeAddressSpace(A) || A == LangAS::Default) &&
 (isPtrSizeAddressSpace(B) || B == LangAS::Default))

Mainly I wanted to avoid recomputing isPtrSizeAddressSpace for A and B.

I think it's only not equivalent when A and B are both default, but we already return true in that case.


This code should be unreachable because you check for these address spaces at the call site. I think you can do something like this:

case LangAS::...:
case LangAS::...:
case LangAS::...:
  llvm_unreachable("don't mangle ptr address spaces with _AS");

Think we should say "__sptr __ptr32"? This code doesn't guarantee that it can be parsed back as valid source, but it's closer.


I wonder if the simplest way to express this would be to follow the pattern of getFunctionTypeWithExceptionSpec and hasSameFunctionTypeIgnoringExceptionSpec, i.e. make a function that strips pointer sized address spaces off of pointer typed arguments, returns it, and then compare them. ASTContext would be a natural place for that kind of type adjustment.


I think it would be fair to raise this method up to ASTContext, next to getAddrSpaceQualType, removeAddrSpaceQualType, etc.

Can you also add documentation to the attribute in and hook it up to the attribute in now that we're actually processing these attributes rather than ignoring them?

nhaehnle removed a subscriber: nhaehnle.Dec 13 2019, 5:43 AM
akhuang updated this revision to Diff 234125.Dec 16 2019, 1:05 PM
akhuang marked 8 inline comments as done.
  • Added docs for ptr32, ptr64, sptr, utr
  • Moved some functions into ASTContext
  • and addressed other comments

Yes -- I think I considered doing this and then forgot that we already return true when A and B are both default.


Done, this does make the code a bit shorter.

rnk accepted this revision.Dec 16 2019, 3:08 PM

Looks great to me.

This has the potential to break some existing code, though. I would suggest either landing it early in the day, watching for breakage, and hoping for the best, or you could try building an application that makes significant use of Windows SDK headers to get more confidence that we won't have to revert it. You could build the sbox_integration_tests target in Chrome or chrome_elf, and see if that works.


Nice, this version is very simple.

This revision is now accepted and ready to land.Dec 16 2019, 3:08 PM
This revision was automatically updated to reflect the committed changes.
phosek added a subscriber: phosek.Dec 18 2019, 12:37 PM

This seems to be failing on aarch64-linux-gnu:

******************** TEST 'Clang :: CodeGenCXX/mangle-ptr-size-address-space.cpp' FAILED ********************
: 'RUN: at line 1';   /b/s/w/ir/k/recipe_cleanup/clangdgOoVq/llvm_build_dir/bin/clang -cc1 -internal-isystem /b/s/w/ir/k/recipe_cleanup/clangdgOoVq/llvm_build_dir/lib/clang/10.0.0/include -nostdsysteminc -fms-extensions -emit-llvm -triple aarch64-unknown-linux-gnu -o - /b/s/w/ir/k/llvm-project/clang/test/CodeGenCXX/mangle-ptr-size-address-space.cpp | /b/s/w/ir/k/recipe_cleanup/clangdgOoVq/llvm_build_dir/bin/FileCheck /b/s/w/ir/k/llvm-project/clang/test/CodeGenCXX/mangle-ptr-size-address-space.cpp --check-prefixes=CHECK
: 'RUN: at line 2';   /b/s/w/ir/k/recipe_cleanup/clangdgOoVq/llvm_build_dir/bin/clang -cc1 -internal-isystem /b/s/w/ir/k/recipe_cleanup/clangdgOoVq/llvm_build_dir/lib/clang/10.0.0/include -nostdsysteminc -fms-extensions -emit-llvm -triple x86_64-windows-msvc -o - /b/s/w/ir/k/llvm-project/clang/test/CodeGenCXX/mangle-ptr-size-address-space.cpp | /b/s/w/ir/k/recipe_cleanup/clangdgOoVq/llvm_build_dir/bin/FileCheck /b/s/w/ir/k/llvm-project/clang/test/CodeGenCXX/mangle-ptr-size-address-space.cpp --check-prefixes=WIN
Exit Code: 1

Command Output (stderr):
/b/s/w/ir/k/llvm-project/clang/test/CodeGenCXX/mangle-ptr-size-address-space.cpp:8:17: error: CHECK-LABEL: expected string not found in input
// CHECK-LABEL: define {{.*}}i8 addrspace(271)* @_Z2f1PU10ptr32_sptri
<stdin>:6:34: note: scanning from here
define void @_Z2f0PU10ptr32_sptri(i32* %p) #0 {
<stdin>:13:1: note: possible intended match here
define i8* @_Z2f1PU10ptr32_sptri(i32* %p) #0 {



The full output is here: