[PATCH] D119816: Fix not working attribute no_sanitize bounds that affects linux kernel

2022-02-15 Thread Tong Zhang via Phabricator via cfe-commits 
ztong0001 created this revision.
Herald added subscribers: ormris, dexonsmith, jdoerfert, steven_wu, hiraditya.
ztong0001 requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

Adding __attribute__((no_sanitize("bounds"))) is not really disabling bounds 
checking, i.e. disable -fsanitize=array-bounds -fsanitize=local-bounds.  or 
-fsanitize=bounds.
We have a potential user in Linux kennel which is currently affected by 
-fsanitize=local-bounds.

Linux kernel(5.17) has a function pskb_expand_head (net/core/skbuff.c) 
currently experiencing false positive when CONFIG_UBSAN_LOCAL_BOUNDS is enabled 
(i.e. supply -fsanitize=array-bounds -fsanitize=local-bounds).

The aforementioned function does the following which will create a false 
positive.

  char* p = kmalloc(M); // allocate buffer, size is M
  int actual_size = ksize(p); // return actual allocated size in the slab 
allocator, this will guaranteed to be larger than M + N
  p[actual_size - N+1] = 0; //Out of bound access

LLVM assume the size of the buffer is M, thus the later p[actual_size-N+1] 
access will be treated as out of bound access.
However, this function deliberately use the actual allocated size instead of M.
Kernel will be unhappy as bounds check fail and it crash hard with the 
following message:

[0.060423] PTP clock support registered
[0.060820] invalid opcode:  [#1] PREEMPT SMP NOPTI
[0.061232] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 
5.17.0-rc3-00247-g83e396641110-dirty #102
[0.061418] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 
1.13.0-1ubuntu1.1 04/01/2014
[0.061418] RIP: 0010:pskb_expand_head+0x38b/0x3b0
[0.061418] Code: 84 fc fd ff ff be 01 00 00 00 b8 01 00 00 00 3e 0f c1 47 
18 85 c0 74 14 8d 48 01 0
9 c1 0f 89 de fd ff ff eb 0c 67 0f b9 40 12 <0f> 0b be 02 00 00 00 48 83 c7 18 
e8 25 b7 cc ff e9 c2 fd
ff ff 0f
[0.061418] RSP: 0018:c900bb60 EFLAGS: 00010287
[0.061418] RAX: 06c0 RBX: 888002e92000 RCX: 
[0.061418] RDX: 06c0 RSI: 888002cebef0 RDI: 0002c800
[0.061418] RBP: 888002cebec0 R08: 888002ceb000 R09: 8174ed1f
[0.061418] R10 : 
ea0b3a00 R11 : 
 R12 : 888002ceb000
[0.061418] R13 :  
R14 : 00012a20 R15 
: 888002e0f000
[0.061418] FS:  () GS:88803ec0() 
knlGS:
[0.061418] CS:  0010 DS:  ES:  CR0: 80050033
[0.061418] CR2: 888002a01000 CR3: 0220c000 CR4: 00350ef0
[0.061418] Call Trace:
[0.061418]  
[0.061418]  netlink_trim+0x83/0xa0
[0.061418]  netlink_broadcast+0x2a/0x5a0
[0.061418]  ? nla_put+0x72/0x90
[0.061418]  genlmsg_multicast_allns+0xcc/0x120
[0.061418]  genl_ctrl_event+0x219/0x370
[0.061418]  genl_register_family+0x5b8/0x660
[0.061418]  ? rtnl_register_internal+0x14d/0x180
[0.061418]  ? tca_get_fill+0x120/0x120
[0.061418]  ? tc_ctl_action+0x5b0/0x5b0
[0.061418]  ? genl_init+0x31/0x31
[0.061418]  ethnl_init+0xd/0x50
[0.061418]  do_one_initcall+0xa5/0x290
[0.061418]  ? alloc_page_interleave+0x5e/0x80
[0.061418]  ? preempt_count_add+0x5e/0xa0
[0.061418]  ? ida_alloc_range+0x3fc/0x440
[0.061418]  ? parse_args+0x254/0x390
[0.061418]  do_initcall_level+0xb4/0x131
[0.061418]  do_initcalls+0x44/0x6b
[0.061418]  kernel_init_freeable+0xd8/0x138
[0.061418]  ? rest_init+0xc0/0xc0
[0.061418]  kernel_init+0x11/0x1a0
[0.061418]  ret_from_fork+0x1f/0x30

The solution here is to add __attribute__((no_sanitize("bounds"))) to this 
function to avoid local bounds checking altogether.

This patch adds __attribute__((no_sanitize("bounds"))) support to LLVM, and in 
linux kernel, we will add this attribute to the function definition.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D119816

Files:
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/test/CodeGen/sanitize-coverage.c
  llvm/include/llvm/AsmParser/LLToken.h
  llvm/include/llvm/Bitcode/LLVMBitCodes.h
  llvm/include/llvm/IR/Attributes.td
  llvm/lib/Bitcode/Reader/BitcodeReader.cpp
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/Transforms/Instrumentation/BoundsChecking.cpp
  llvm/lib/Transforms/Utils/CodeExtractor.cpp

Index: llvm/lib/Transforms/Utils/CodeExtractor.cpp
===
--- llvm/lib/Transforms/Utils/CodeExtractor.cpp
+++ llvm/lib/Transforms/Utils/CodeExtractor.cpp
@@ -938,6 +938,7 @@
   case Attribute::NonLazyBind:
   case Attribute::NoRedZone:
   case Attribute::NoUnwind:
+  case

[PATCH] D119816: Fix not working attribute no_sanitize bounds that affects linux kernel

2022-02-18 Thread Tong Zhang via Phabricator via cfe-commits 
ztong0001 added a comment.

In D119816#3331658 , @nlopes wrote:

> Well, this patch is just a band-aid and a disaster waiting to happen.
> If kmalloc is tagged with an `__attribute__` stating the allocation size, 
> then you can't dereference beyond that limit or risk the alias analysis do 
> something you don't want. You are triggering UB like that.
> Can't you just remove the `__attribute__` from kmalloc instead to avoid 
> triggering UB?

I am not sure I fully get what you are saying.
What I am suggesting is something like below to avoid bounds check pass from 
running on this function.

  diff --git a/net/core/skbuff.c b/net/core/skbuff.c
  index 9d0388bed0c1..186fca35266d 100644
  --- a/net/core/skbuff.c
  +++ b/net/core/skbuff.c
  @@ -1679,7 +1679,7 @@ EXPORT_SYMBOL(__pskb_copy_fclone);
* All the pointers pointing into skb header may change and must be
* reloaded after call to this function.
*/
  -
  +__attribute__((no_sanitize("bounds")))
   int pskb_expand_head(struct sk_buff *skb, int nhead, int ntail,
   gfp_t gfp_mask)
   {


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119816/new/

https://reviews.llvm.org/D119816

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D119816: Fix not working attribute no_sanitize bounds that affects linux kernel

2022-02-18 Thread Tong Zhang via Phabricator via cfe-commits 
ztong0001 added a comment.

In D119816#3331797 , @melver wrote:

> Right, I was able to repro this. The problem is the trap, which generally 
> sucks that no_sanitize still leaves in the trap.
>
> We also have -fno-sanitize-undefined-trap-on-error, which seems to have no 
> effect either (should it?).
>
> So I think there are 2 problems:
>
> 1. Clang still emitting traps even though it shouldn't.
>
> 2. The Linux kernel problem.
>
> I think it's fine if you address problem 1 with this, as it's an oversight. 
> But I think problem 2 wants to be solved differently as I suggested.

I haven't tried -fno-sanitize-undefined-trap-on-error yet.

IMO trap in kernel gives a generic crash message which is... hard to tell from 
other cases without further investigating. If I enable KASAN kernel will print 
out something like

`
[1.197953] BUG: KASAN: use-after-free in __pci_enable_msi_range+0x234/0x320
[1.198327] Freed by task 1:
[1.198327]  kfree+0x8f/0x2b0
[1.198327]  msi_free_msi_descs_range+0xf5/0x130
`

I agree with you that there are two problems.
I think it makes sense to let optimizer aware of `ksize()` if the kernel API 
won't change dramatically in the future.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119816/new/

https://reviews.llvm.org/D119816

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D119816: Fix not working attribute no_sanitize bounds that affects linux kernel

2022-02-18 Thread Tong Zhang via Phabricator via cfe-commits 
ztong0001 added a comment.

In D119816#3332441 , @nlopes wrote:

> The main issue is that the kernel is wrong. It has a bug. The sanitizer's 
> error is not a false-positive!
> So what you are proposing is a band-aid. It's not a real solution and it's 
> just masking a wider problem. LLVM knows that kmalloc(x) allocates x bytes 
> because someone placed an `__attribute__ ((alloc_size (1)))` on kmalloc. That 
> attribute is just wrong and should be removed. It allows LLVM to mark all 
> accesses beyond `kmalloc(x) + x - 1` as undefined behavior.

But isn't this something the author intended to do in order to catch an error? 
`ksize()` case makes some exceptions out of this.

> TL;DR: this patch is not the solution for your problems.




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119816/new/

https://reviews.llvm.org/D119816

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D119816: Fix not working attribute no_sanitize bounds that affects linux kernel

2022-02-18 Thread Tong Zhang via Phabricator via cfe-commits 
ztong0001 added a comment.

Thank you all!
I have modified the summary, this patch focus on fixing non-working 
`__attribute__((no_sanitize("bounds"))) ` attribute.
I will try to fix kernel `ksize()` related issue and 
`-fno-sanitize-undefined-trap-on-error` on separate patches.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119816/new/

https://reviews.llvm.org/D119816

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D119816: Fix not working attribute no_sanitize bounds that affects linux kernel

2022-02-22 Thread Tong Zhang via Phabricator via cfe-commits 
ztong0001 added a comment.

Thank you @melver. I will revise patch as suggested.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119816/new/

https://reviews.llvm.org/D119816

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D119816: Fix not working attribute no_sanitize bounds that affects linux kernel

2022-02-24 Thread Tong Zhang via Phabricator via cfe-commits 
ztong0001 updated this revision to Diff 411040.
ztong0001 edited the summary of this revision.
ztong0001 added a comment.

update patch and description as suggested by Marco


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119816/new/

https://reviews.llvm.org/D119816

Files:
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/test/CodeGen/sanitize-coverage.c
  llvm/bindings/go/llvm/ir_test.go
  llvm/docs/BitCodeFormat.rst
  llvm/docs/LangRef.rst
  llvm/include/llvm/AsmParser/LLToken.h
  llvm/include/llvm/Bitcode/LLVMBitCodes.h
  llvm/include/llvm/IR/Attributes.td
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/Bitcode/Reader/BitcodeReader.cpp
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/Transforms/Instrumentation/BoundsChecking.cpp
  llvm/lib/Transforms/Utils/CodeExtractor.cpp
  llvm/test/Bitcode/attributes.ll
  llvm/test/Bitcode/compatibility.ll
  llvm/utils/emacs/llvm-mode.el
  llvm/utils/llvm.grm
  llvm/utils/vim/syntax/llvm.vim
  llvm/utils/vscode/llvm/syntaxes/ll.tmLanguage.yaml

Index: llvm/utils/vscode/llvm/syntaxes/ll.tmLanguage.yaml
===
--- llvm/utils/vscode/llvm/syntaxes/ll.tmLanguage.yaml
+++ llvm/utils/vscode/llvm/syntaxes/ll.tmLanguage.yaml
@@ -238,6 +238,7 @@
 \\bnosync\\b|\
 \\bnoundef\\b|\
 \\bnounwind\\b|\
+\\bnosanitize_bounds\\b|\
 \\bnosanitize_coverage\\b|\
 \\bnull_pointer_is_valid\\b|\
 \\boptforfuzzing\\b|\
Index: llvm/utils/vim/syntax/llvm.vim
===
--- llvm/utils/vim/syntax/llvm.vim
+++ llvm/utils/vim/syntax/llvm.vim
@@ -139,6 +139,7 @@
   \ nosync
   \ noundef
   \ nounwind
+  \ nosanitize_bounds
   \ nosanitize_coverage
   \ null_pointer_is_valid
   \ optforfuzzing
Index: llvm/utils/llvm.grm
===
--- llvm/utils/llvm.grm
+++ llvm/utils/llvm.grm
@@ -176,6 +176,7 @@
  | sanitize_thread
  | sanitize_memory
  | mustprogress
+ | nosanitize_bounds
  | nosanitize_coverage
  ;

Index: llvm/utils/emacs/llvm-mode.el
===
--- llvm/utils/emacs/llvm-mode.el
+++ llvm/utils/emacs/llvm-mode.el
@@ -25,7 +25,7 @@
'("alwaysinline" "argmemonly" "allocsize" "builtin" "cold" "convergent" "dereferenceable" "dereferenceable_or_null" "hot" "inaccessiblememonly"
  "inaccessiblemem_or_argmemonly" "inalloca" "inlinehint" "jumptable" "minsize" "mustprogress" "naked" "nobuiltin" "nonnull"
  "nocallback" "nocf_check" "noduplicate" "nofree" "noimplicitfloat" "noinline" "nomerge" "nonlazybind" "noprofile" "noredzone" "noreturn"
- "norecurse" "nosync" "noundef" "nounwind" "nosanitize_coverage" "null_pointer_is_valid" "optforfuzzing" "optnone" "optsize" "preallocated" "readnone" "readonly" "returned" "returns_twice"
+ "norecurse" "nosync" "noundef" "nounwind" "nosanitize_bounds" "nosanitize_coverage" "null_pointer_is_valid" "optforfuzzing" "optnone" "optsize" "preallocated" "readnone" "readonly" "returned" "returns_twice"
  "shadowcallstack" "speculatable" "speculative_load_hardening" "ssp" "sspreq" "sspstrong" "safestack" "sanitize_address" "sanitize_hwaddress" "sanitize_memtag"
  "sanitize_thread" "sanitize_memory" "strictfp" "swifterror" "uwtable" "vscale_range" "willreturn" "writeonly" "immarg") 'symbols) . font-lock-constant-face)
;; Variables
Index: llvm/test/Bitcode/compatibility.ll
===
--- llvm/test/Bitcode/compatibility.ll
+++ llvm/test/Bitcode/compatibility.ll
@@ -1510,7 +1510,7 @@
   ; CHECK: select <2 x i1> , <2 x i8> , <2 x i8> 

   call void @f.nobuiltin() builtin
-  ; CHECK: call void @f.nobuiltin() #48
+  ; CHECK: call void @f.nobuiltin() #49

   call fastcc noalias i32* @f.noalias() noinline
   ; CHECK: call fastcc noalias i32* @f.noalias() #12
@@ -1930,6 +1930,9 @@
 ; CHECK: Function Attrs: allocsize(1,0)
 ; CHECK: declare void @f.allocsize_two(i32, i32)

+declare void @f.nosanitize_bounds() nosanitize_bounds
+; CHECK: declare void @f.nosanitize_bounds() #48
+
 ; CHECK: attributes #0 = { alignstack=4 }
 ; CHECK: attributes #1 = { alignstack=8 }
 ; CHECK: attributes #2 = { alwaysinline }
@@ -1978,7 +1981,8 @@
 ; CHECK: attributes #45 = { disable_sanitizer_instrumentation }
 ; CHECK: attributes #46 = { allocsize(0) }
 ; CHECK: attributes #47 = { allocsize(1,0) }
-; CHECK: attributes #48 = { builtin }
+; CHECK: attributes #48 = { nosanitize_bounds }
+; CHECK: attributes #49 = { builtin }

 ;; Metadata

Index: llvm/test/Bitcode/attributes.ll
===
--- llvm/test/Bitcode/attributes.ll
+++ llvm/test/Bitcode/attributes.ll
@@ -526,6 +526,12 @@
 ret void;
 }

+; CHECK: define void @f86() #52
+define void @f86()

[PATCH] D119816: [SanitizerBounds] Add support for NoSanitizeBounds function

2022-02-24 Thread Tong Zhang via Phabricator via cfe-commits 
ztong0001 updated this revision to Diff 411191.
ztong0001 added a comment.

Thank you, Marco! I have made the following changes:

- extend clang/test/CodeGen/bounds-checking.c to include additional test for 
newly added nosanitize_bounds attribute
- added a new pure IR test in 
llvm/test/Instrumentation/BoundsChecking/nosanitize-bounds.ll to test if the 
new attribute will prevent BoundsChecking pass running on the function


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119816/new/

https://reviews.llvm.org/D119816

Files:
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/test/CodeGen/bounds-checking.c
  clang/test/CodeGen/sanitize-coverage.c
  llvm/bindings/go/llvm/ir_test.go
  llvm/docs/BitCodeFormat.rst
  llvm/docs/LangRef.rst
  llvm/include/llvm/AsmParser/LLToken.h
  llvm/include/llvm/Bitcode/LLVMBitCodes.h
  llvm/include/llvm/IR/Attributes.td
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/Bitcode/Reader/BitcodeReader.cpp
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/Transforms/Instrumentation/BoundsChecking.cpp
  llvm/lib/Transforms/Utils/CodeExtractor.cpp
  llvm/test/Bitcode/attributes.ll
  llvm/test/Bitcode/compatibility.ll
  llvm/test/Instrumentation/BoundsChecking/nosanitize-bounds.ll
  llvm/utils/emacs/llvm-mode.el
  llvm/utils/llvm.grm
  llvm/utils/vim/syntax/llvm.vim
  llvm/utils/vscode/llvm/syntaxes/ll.tmLanguage.yaml

Index: llvm/utils/vscode/llvm/syntaxes/ll.tmLanguage.yaml
===
--- llvm/utils/vscode/llvm/syntaxes/ll.tmLanguage.yaml
+++ llvm/utils/vscode/llvm/syntaxes/ll.tmLanguage.yaml
@@ -238,6 +238,7 @@
 \\bnosync\\b|\
 \\bnoundef\\b|\
 \\bnounwind\\b|\
+\\bnosanitize_bounds\\b|\
 \\bnosanitize_coverage\\b|\
 \\bnull_pointer_is_valid\\b|\
 \\boptforfuzzing\\b|\
Index: llvm/utils/vim/syntax/llvm.vim
===
--- llvm/utils/vim/syntax/llvm.vim
+++ llvm/utils/vim/syntax/llvm.vim
@@ -139,6 +139,7 @@
   \ nosync
   \ noundef
   \ nounwind
+  \ nosanitize_bounds
   \ nosanitize_coverage
   \ null_pointer_is_valid
   \ optforfuzzing
Index: llvm/utils/llvm.grm
===
--- llvm/utils/llvm.grm
+++ llvm/utils/llvm.grm
@@ -176,6 +176,7 @@
  | sanitize_thread
  | sanitize_memory
  | mustprogress
+ | nosanitize_bounds
  | nosanitize_coverage
  ;

Index: llvm/utils/emacs/llvm-mode.el
===
--- llvm/utils/emacs/llvm-mode.el
+++ llvm/utils/emacs/llvm-mode.el
@@ -25,7 +25,7 @@
'("alwaysinline" "argmemonly" "allocsize" "builtin" "cold" "convergent" "dereferenceable" "dereferenceable_or_null" "hot" "inaccessiblememonly"
  "inaccessiblemem_or_argmemonly" "inalloca" "inlinehint" "jumptable" "minsize" "mustprogress" "naked" "nobuiltin" "nonnull"
  "nocallback" "nocf_check" "noduplicate" "nofree" "noimplicitfloat" "noinline" "nomerge" "nonlazybind" "noprofile" "noredzone" "noreturn"
- "norecurse" "nosync" "noundef" "nounwind" "nosanitize_coverage" "null_pointer_is_valid" "optforfuzzing" "optnone" "optsize" "preallocated" "readnone" "readonly" "returned" "returns_twice"
+ "norecurse" "nosync" "noundef" "nounwind" "nosanitize_bounds" "nosanitize_coverage" "null_pointer_is_valid" "optforfuzzing" "optnone" "optsize" "preallocated" "readnone" "readonly" "returned" "returns_twice"
  "shadowcallstack" "speculatable" "speculative_load_hardening" "ssp" "sspreq" "sspstrong" "safestack" "sanitize_address" "sanitize_hwaddress" "sanitize_memtag"
  "sanitize_thread" "sanitize_memory" "strictfp" "swifterror" "uwtable" "vscale_range" "willreturn" "writeonly" "immarg") 'symbols) . font-lock-constant-face)
;; Variables
Index: llvm/test/Instrumentation/BoundsChecking/nosanitize-bounds.ll
===
--- /dev/null
+++ llvm/test/Instrumentation/BoundsChecking/nosanitize-bounds.ll
@@ -0,0 +1,17 @@
+; RUN: opt < %s -passes=bounds-checking -S | FileCheck %s
+target datalayout = "e-p:64:64:64-p1:16:16:16-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
+
+; CHECK: @foo
+define i32 @foo(i32 %i) nosanitize_bounds {
+entry:
+  %i.addr = alloca i32, align 4
+  %b = alloca [64 x i32], align 16
+  store i32 %i, i32* %i.addr, align 4
+  %0 = load i32, i32* %i.addr, align 4
+  %idxprom = sext i32 %0 to i64
+  %arrayidx = getelementptr inbounds [64 x i32], [64 x i32]* %b, i64 0, i64 %idxprom
+  %1 = load i32, i32* %arrayidx, align 4
+  ret i32 %1
+; CHECK-NOT: call void @llvm.trap()
+}
+
Index: llvm/test/Bitcode/compatibility.ll
===
--- llvm/test/Bitcode/compatibility.ll
+++ llvm/test/

[PATCH] D119816: [SanitizerBounds] Add support for NoSanitizeBounds function

2022-02-25 Thread Tong Zhang via Phabricator via cfe-commits 
ztong0001 added a comment.

In D119816#3345302 , @melver wrote:

> Looks good. Few minor changes.
>
> I did some more digging, and it's only fsanitize=local-bounds, so please 
> verify this and also update the commit description. In fact, the Linux kernel 
> already has a comment about Clang's weirdness of local-bounds vs. 
> array-bounds here: 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/lib/Kconfig.ubsan#n62

Yes I can confirm this. It is indeed fsanitize=local-bounds causing the kernel 
crash we discussed earlier. (UBSAN_LOCAL_BOUNDS=y)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119816/new/

https://reviews.llvm.org/D119816

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D119816: [SanitizerBounds] Add support for NoSanitizeBounds function

2022-02-25 Thread Tong Zhang via Phabricator via cfe-commits 
ztong0001 added inline comments.



Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:757
 SanOpts.set(SanitizerKind::HWAddress, false);
+  if (mask & SanitizerKind::LocalBounds)
+Fn->addFnAttr(llvm::Attribute::NoSanitizeBounds);

melver wrote:
> These 2 checks can be reduced to 1 due to SanitizerKind::Bounds including 
> both. However, as noted, this only affects local-bounds, so I'm not sure if 
> we want to include both -- perhaps for completeness it makes sense, but in 
> the array-bounds only case this attribute will be a nop (AFAIK).
> 
> Also, I think we don't want to attach the attribute if bounds checking isn't 
> enabled -- at least it seems unnecessary to do so.
> 
> See the following suggested change:
> 
> ```
> diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp 
> b/clang/lib/CodeGen/CodeGenFunction.cpp
> index 842ce0245d45..c1f3a3014a19 100644
> --- a/clang/lib/CodeGen/CodeGenFunction.cpp
> +++ b/clang/lib/CodeGen/CodeGenFunction.cpp
> @@ -740,6 +740,7 @@ void CodeGenFunction::StartFunction(GlobalDecl GD, 
> QualType RetTy,
>} while (false);
>  
>if (D) {
> +const bool SanitizeBounds = SanOpts.hasOneOf(SanitizerKind::Bounds);
>  bool NoSanitizeCoverage = false;
>  
>  for (auto Attr : D->specific_attrs()) {
> @@ -754,16 +755,15 @@ void CodeGenFunction::StartFunction(GlobalDecl GD, 
> QualType RetTy,
>  SanOpts.set(SanitizerKind::KernelHWAddress, false);
>if (mask & SanitizerKind::KernelHWAddress)
>  SanOpts.set(SanitizerKind::HWAddress, false);
> -  if (mask & SanitizerKind::LocalBounds)
> -Fn->addFnAttr(llvm::Attribute::NoSanitizeBounds);
> -  if (mask & SanitizerKind::ArrayBounds)
> -Fn->addFnAttr(llvm::Attribute::NoSanitizeBounds);
>  
>// SanitizeCoverage is not handled by SanOpts.
>if (Attr->hasCoverage())
>  NoSanitizeCoverage = true;
>  }
>  
> +if (SanitizeBounds && !SanOpts.hasOneOf(SanitizerKind::Bounds))
> +  Fn->addFnAttr(llvm::Attribute::NoSanitizeBounds);
> +
>  if (NoSanitizeCoverage && CGM.getCodeGenOpts().hasSanitizeCoverage())
>Fn->addFnAttr(llvm::Attribute::NoSanitizeCoverage);
>}
> 
> ```
Agreed. I will revise patch and commit description.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119816/new/

https://reviews.llvm.org/D119816

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D119816: [SanitizerBounds] Add support for NoSanitizeBounds function

2022-02-25 Thread Tong Zhang via Phabricator via cfe-commits 
ztong0001 added inline comments.



Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:757
 SanOpts.set(SanitizerKind::HWAddress, false);
+  if (mask & SanitizerKind::LocalBounds)
+Fn->addFnAttr(llvm::Attribute::NoSanitizeBounds);

ztong0001 wrote:
> melver wrote:
> > These 2 checks can be reduced to 1 due to SanitizerKind::Bounds including 
> > both. However, as noted, this only affects local-bounds, so I'm not sure if 
> > we want to include both -- perhaps for completeness it makes sense, but in 
> > the array-bounds only case this attribute will be a nop (AFAIK).
> > 
> > Also, I think we don't want to attach the attribute if bounds checking 
> > isn't enabled -- at least it seems unnecessary to do so.
> > 
> > See the following suggested change:
> > 
> > ```
> > diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp 
> > b/clang/lib/CodeGen/CodeGenFunction.cpp
> > index 842ce0245d45..c1f3a3014a19 100644
> > --- a/clang/lib/CodeGen/CodeGenFunction.cpp
> > +++ b/clang/lib/CodeGen/CodeGenFunction.cpp
> > @@ -740,6 +740,7 @@ void CodeGenFunction::StartFunction(GlobalDecl GD, 
> > QualType RetTy,
> >} while (false);
> >  
> >if (D) {
> > +const bool SanitizeBounds = SanOpts.hasOneOf(SanitizerKind::Bounds);
> >  bool NoSanitizeCoverage = false;
> >  
> >  for (auto Attr : D->specific_attrs()) {
> > @@ -754,16 +755,15 @@ void CodeGenFunction::StartFunction(GlobalDecl GD, 
> > QualType RetTy,
> >  SanOpts.set(SanitizerKind::KernelHWAddress, false);
> >if (mask & SanitizerKind::KernelHWAddress)
> >  SanOpts.set(SanitizerKind::HWAddress, false);
> > -  if (mask & SanitizerKind::LocalBounds)
> > -Fn->addFnAttr(llvm::Attribute::NoSanitizeBounds);
> > -  if (mask & SanitizerKind::ArrayBounds)
> > -Fn->addFnAttr(llvm::Attribute::NoSanitizeBounds);
> >  
> >// SanitizeCoverage is not handled by SanOpts.
> >if (Attr->hasCoverage())
> >  NoSanitizeCoverage = true;
> >  }
> >  
> > +if (SanitizeBounds && !SanOpts.hasOneOf(SanitizerKind::Bounds))
> > +  Fn->addFnAttr(llvm::Attribute::NoSanitizeBounds);
> > +
> >  if (NoSanitizeCoverage && CGM.getCodeGenOpts().hasSanitizeCoverage())
> >Fn->addFnAttr(llvm::Attribute::NoSanitizeCoverage);
> >}
> > 
> > ```
> Agreed. I will revise patch and commit description.
BTW: Just to double check we are on the same page

when no_sanitize(bounds) is provided, fsanitize=array-bounds is also disabled 
-- right ? I can confirm this attribute is also affecting array-bounds as this 
is handled in clang side and we are setting llvm::Attribute::NoSanitizeBounds 
so that clang's array-bounds can also see this.

I will also add another test for -fsanitize=array-bounds


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119816/new/

https://reviews.llvm.org/D119816

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D119816: [SanitizerBounds] Add support for NoSanitizeBounds function

2022-02-25 Thread Tong Zhang via Phabricator via cfe-commits 
ztong0001 updated this revision to Diff 411457.
ztong0001 added a comment.

- update commit description
- In: CodeGenFunction::StartFunction(), merge two 
checks(SanitizerKind::LocalBounds, SanitizerKind::ArrayBounds) into 
one(SanitizerKind::Bounds)
- update test: clang/test/CodeGen/bounds-checking.c to show 
no_sanitize("bounds") can also affect fsanitize=array-bounds (clang)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119816/new/

https://reviews.llvm.org/D119816

Files:
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/test/CodeGen/bounds-checking.c
  clang/test/CodeGen/sanitize-coverage.c
  llvm/bindings/go/llvm/ir_test.go
  llvm/docs/BitCodeFormat.rst
  llvm/docs/LangRef.rst
  llvm/include/llvm/AsmParser/LLToken.h
  llvm/include/llvm/Bitcode/LLVMBitCodes.h
  llvm/include/llvm/IR/Attributes.td
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/Bitcode/Reader/BitcodeReader.cpp
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/Transforms/Instrumentation/BoundsChecking.cpp
  llvm/lib/Transforms/Utils/CodeExtractor.cpp
  llvm/test/Bitcode/attributes.ll
  llvm/test/Bitcode/compatibility.ll
  llvm/test/Instrumentation/BoundsChecking/nosanitize-bounds.ll
  llvm/utils/emacs/llvm-mode.el
  llvm/utils/llvm.grm
  llvm/utils/vim/syntax/llvm.vim
  llvm/utils/vscode/llvm/syntaxes/ll.tmLanguage.yaml

Index: llvm/utils/vscode/llvm/syntaxes/ll.tmLanguage.yaml
===
--- llvm/utils/vscode/llvm/syntaxes/ll.tmLanguage.yaml
+++ llvm/utils/vscode/llvm/syntaxes/ll.tmLanguage.yaml
@@ -238,6 +238,7 @@
 \\bnosync\\b|\
 \\bnoundef\\b|\
 \\bnounwind\\b|\
+\\bnosanitize_bounds\\b|\
 \\bnosanitize_coverage\\b|\
 \\bnull_pointer_is_valid\\b|\
 \\boptforfuzzing\\b|\
Index: llvm/utils/vim/syntax/llvm.vim
===
--- llvm/utils/vim/syntax/llvm.vim
+++ llvm/utils/vim/syntax/llvm.vim
@@ -139,6 +139,7 @@
   \ nosync
   \ noundef
   \ nounwind
+  \ nosanitize_bounds
   \ nosanitize_coverage
   \ null_pointer_is_valid
   \ optforfuzzing
Index: llvm/utils/llvm.grm
===
--- llvm/utils/llvm.grm
+++ llvm/utils/llvm.grm
@@ -176,6 +176,7 @@
  | sanitize_thread
  | sanitize_memory
  | mustprogress
+ | nosanitize_bounds
  | nosanitize_coverage
  ;

Index: llvm/utils/emacs/llvm-mode.el
===
--- llvm/utils/emacs/llvm-mode.el
+++ llvm/utils/emacs/llvm-mode.el
@@ -25,7 +25,7 @@
'("alwaysinline" "argmemonly" "allocsize" "builtin" "cold" "convergent" "dereferenceable" "dereferenceable_or_null" "hot" "inaccessiblememonly"
  "inaccessiblemem_or_argmemonly" "inalloca" "inlinehint" "jumptable" "minsize" "mustprogress" "naked" "nobuiltin" "nonnull"
  "nocallback" "nocf_check" "noduplicate" "nofree" "noimplicitfloat" "noinline" "nomerge" "nonlazybind" "noprofile" "noredzone" "noreturn"
- "norecurse" "nosync" "noundef" "nounwind" "nosanitize_coverage" "null_pointer_is_valid" "optforfuzzing" "optnone" "optsize" "preallocated" "readnone" "readonly" "returned" "returns_twice"
+ "norecurse" "nosync" "noundef" "nounwind" "nosanitize_bounds" "nosanitize_coverage" "null_pointer_is_valid" "optforfuzzing" "optnone" "optsize" "preallocated" "readnone" "readonly" "returned" "returns_twice"
  "shadowcallstack" "speculatable" "speculative_load_hardening" "ssp" "sspreq" "sspstrong" "safestack" "sanitize_address" "sanitize_hwaddress" "sanitize_memtag"
  "sanitize_thread" "sanitize_memory" "strictfp" "swifterror" "uwtable" "vscale_range" "willreturn" "writeonly" "immarg") 'symbols) . font-lock-constant-face)
;; Variables
Index: llvm/test/Instrumentation/BoundsChecking/nosanitize-bounds.ll
===
--- /dev/null
+++ llvm/test/Instrumentation/BoundsChecking/nosanitize-bounds.ll
@@ -0,0 +1,17 @@
+; RUN: opt < %s -passes=bounds-checking -S | FileCheck %s
+target datalayout = "e-p:64:64:64-p1:16:16:16-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
+
+; CHECK: @foo
+define i32 @foo(i32 %i) nosanitize_bounds {
+entry:
+  %i.addr = alloca i32, align 4
+  %b = alloca [64 x i32], align 16
+  store i32 %i, i32* %i.addr, align 4
+  %0 = load i32, i32* %i.addr, align 4
+  %idxprom = sext i32 %0 to i64
+  %arrayidx = getelementptr inbounds [64 x i32], [64 x i32]* %b, i64 0, i64 %idxprom
+  %1 = load i32, i32* %arrayidx, align 4
+  ret i32 %1
+; CHECK-NOT: call void @llvm.trap()
+}
+
Index: llvm/test/Bitcode/compatibility.ll
===
--- llvm/test/Bitcode/compatibility.ll
+++ llvm/test/Bitcode/compatibility.ll
@@ -1510,7 +1510,7 @@

[PATCH] D119816: [SanitizerBounds] Add support for NoSanitizeBounds function

2022-02-25 Thread Tong Zhang via Phabricator via cfe-commits 
ztong0001 added inline comments.



Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:757
 SanOpts.set(SanitizerKind::HWAddress, false);
+  if (mask & SanitizerKind::LocalBounds)
+Fn->addFnAttr(llvm::Attribute::NoSanitizeBounds);

melver wrote:
> ztong0001 wrote:
> > ztong0001 wrote:
> > > melver wrote:
> > > > These 2 checks can be reduced to 1 due to SanitizerKind::Bounds 
> > > > including both. However, as noted, this only affects local-bounds, so 
> > > > I'm not sure if we want to include both -- perhaps for completeness it 
> > > > makes sense, but in the array-bounds only case this attribute will be a 
> > > > nop (AFAIK).
> > > > 
> > > > Also, I think we don't want to attach the attribute if bounds checking 
> > > > isn't enabled -- at least it seems unnecessary to do so.
> > > > 
> > > > See the following suggested change:
> > > > 
> > > > ```
> > > > diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp 
> > > > b/clang/lib/CodeGen/CodeGenFunction.cpp
> > > > index 842ce0245d45..c1f3a3014a19 100644
> > > > --- a/clang/lib/CodeGen/CodeGenFunction.cpp
> > > > +++ b/clang/lib/CodeGen/CodeGenFunction.cpp
> > > > @@ -740,6 +740,7 @@ void CodeGenFunction::StartFunction(GlobalDecl GD, 
> > > > QualType RetTy,
> > > >} while (false);
> > > >  
> > > >if (D) {
> > > > +const bool SanitizeBounds = 
> > > > SanOpts.hasOneOf(SanitizerKind::Bounds);
> > > >  bool NoSanitizeCoverage = false;
> > > >  
> > > >  for (auto Attr : D->specific_attrs()) {
> > > > @@ -754,16 +755,15 @@ void CodeGenFunction::StartFunction(GlobalDecl 
> > > > GD, QualType RetTy,
> > > >  SanOpts.set(SanitizerKind::KernelHWAddress, false);
> > > >if (mask & SanitizerKind::KernelHWAddress)
> > > >  SanOpts.set(SanitizerKind::HWAddress, false);
> > > > -  if (mask & SanitizerKind::LocalBounds)
> > > > -Fn->addFnAttr(llvm::Attribute::NoSanitizeBounds);
> > > > -  if (mask & SanitizerKind::ArrayBounds)
> > > > -Fn->addFnAttr(llvm::Attribute::NoSanitizeBounds);
> > > >  
> > > >// SanitizeCoverage is not handled by SanOpts.
> > > >if (Attr->hasCoverage())
> > > >  NoSanitizeCoverage = true;
> > > >  }
> > > >  
> > > > +if (SanitizeBounds && !SanOpts.hasOneOf(SanitizerKind::Bounds))
> > > > +  Fn->addFnAttr(llvm::Attribute::NoSanitizeBounds);
> > > > +
> > > >  if (NoSanitizeCoverage && 
> > > > CGM.getCodeGenOpts().hasSanitizeCoverage())
> > > >Fn->addFnAttr(llvm::Attribute::NoSanitizeCoverage);
> > > >}
> > > > 
> > > > ```
> > > Agreed. I will revise patch and commit description.
> > BTW: Just to double check we are on the same page
> > 
> > when no_sanitize(bounds) is provided, fsanitize=array-bounds is also 
> > disabled -- right ? I can confirm this attribute is also affecting 
> > array-bounds as this is handled in clang side and we are setting 
> > llvm::Attribute::NoSanitizeBounds so that clang's array-bounds can also see 
> > this.
> > 
> > I will also add another test for -fsanitize=array-bounds
> Well, no_sanitize("bounds") already worked for array-bounds before this 
> patch. But adding more tests never hurt.
Got it. Thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119816/new/

https://reviews.llvm.org/D119816

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D119816: [SanitizerBounds] Add support for NoSanitizeBounds function

2022-02-25 Thread Tong Zhang via Phabricator via cfe-commits 
ztong0001 added a comment.

In D119816#3346575 , @kees wrote:

> FWIW, related problems with `pskb_expand_head` were seen again here:
> https://github.com/ClangBuiltLinux/linux/issues/1599
>
> I have trouble reproducing it, but I think the kernel patch there solves the 
> problem created by `__alloc_size` vs `ksize()`.

yes, the __asm__ ("" : "=r" (p) : "0" (p)); really did the trick


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119816/new/

https://reviews.llvm.org/D119816

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D120596: [clang][CGStmt] fix crash on invalid asm statement

2022-02-25 Thread Tong Zhang via Phabricator via cfe-commits 
ztong0001 created this revision.
Herald added a subscriber: pengfei.
ztong0001 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Clang is crashing on the following statement

  char var[9];
  __asm__ ("" : "=r" (var) : "0" (var));

This is similar to existing test: crbug_999160_regtest

The issue happens when EmitAsmStmt is trying to convert input to match
output type length. However, that is not guaranteed to be successful all the
time and if the statement itself is invalid like having an array type in
the example, we should give a regular error message here instead of
using assert().


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D120596

Files:
  clang/lib/CodeGen/CGStmt.cpp
  clang/test/CodeGen/X86/x86_64-PR42672.c


Index: clang/test/CodeGen/X86/x86_64-PR42672.c
===
--- clang/test/CodeGen/X86/x86_64-PR42672.c
+++ clang/test/CodeGen/X86/x86_64-PR42672.c
@@ -5,6 +5,7 @@
 // RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -DPOSSIBLE_X -emit-llvm %s 
-o - 2>&1 | FileCheck %s --check-prefix=CHECK-X
 // RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_X 
-emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_X
 // RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_9BYTES 
-emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_9BYTES
+// RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_9BYTES_V2 
-emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_9BYTES_V2

 // Make sure Clang doesn't treat |lockval| as asm input.
 void _raw_spin_lock(void) {
@@ -115,3 +116,12 @@
 }

 // CHECK-IMPOSSIBLE_9BYTES: impossible constraint in asm: can't store value 
into a register
+
+void crbug_999160_regtest_v2(void) {
+#ifdef IMPOSSIBLE_9BYTES_V2
+  char buf[9];
+  asm(""
+  : "=r"(buf) : "0"(buf));
+#endif
+}
+// CHECK-IMPOSSIBLE_9BYTES_V2: impossible constraint in asm: can't store value 
into a register
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -2513,10 +2513,8 @@
   Arg = Builder.CreateZExt(Arg, OutputTy);
 else if (isa(OutputTy))
   Arg = Builder.CreateZExt(Arg, IntPtrTy);
-else {
-  assert(OutputTy->isFloatingPointTy() && "Unexpected output type");
+else if (OutputTy->isFloatingPointTy())
   Arg = Builder.CreateFPExt(Arg, OutputTy);
-}
   }
   // Deal with the tied operands' constraint code in adjustInlineAsmType.
   ReplaceConstraint = OutputConstraints[Output];


Index: clang/test/CodeGen/X86/x86_64-PR42672.c
===
--- clang/test/CodeGen/X86/x86_64-PR42672.c
+++ clang/test/CodeGen/X86/x86_64-PR42672.c
@@ -5,6 +5,7 @@
 // RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -DPOSSIBLE_X -emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-X
 // RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_X -emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_X
 // RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_9BYTES -emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_9BYTES
+// RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_9BYTES_V2 -emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_9BYTES_V2

 // Make sure Clang doesn't treat |lockval| as asm input.
 void _raw_spin_lock(void) {
@@ -115,3 +116,12 @@
 }

 // CHECK-IMPOSSIBLE_9BYTES: impossible constraint in asm: can't store value into a register
+
+void crbug_999160_regtest_v2(void) {
+#ifdef IMPOSSIBLE_9BYTES_V2
+  char buf[9];
+  asm(""
+  : "=r"(buf) : "0"(buf));
+#endif
+}
+// CHECK-IMPOSSIBLE_9BYTES_V2: impossible constraint in asm: can't store value into a register
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -2513,10 +2513,8 @@
   Arg = Builder.CreateZExt(Arg, OutputTy);
 else if (isa(OutputTy))
   Arg = Builder.CreateZExt(Arg, IntPtrTy);
-else {
-  assert(OutputTy->isFloatingPointTy() && "Unexpected output type");
+else if (OutputTy->isFloatingPointTy())
   Arg = Builder.CreateFPExt(Arg, OutputTy);
-}
   }
   // Deal with the tied operands' constraint code in adjustInlineAsmType.
   ReplaceConstraint = OutputConstraints[Output];
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D120596: [clang][CGStmt] fix crash on invalid asm statement

2022-02-25 Thread Tong Zhang via Phabricator via cfe-commits 
ztong0001 updated this revision to Diff 411542.
ztong0001 added a comment.

reformat test code


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120596/new/

https://reviews.llvm.org/D120596

Files:
  clang/lib/CodeGen/CGStmt.cpp
  clang/test/CodeGen/X86/x86_64-PR42672.c


Index: clang/test/CodeGen/X86/x86_64-PR42672.c
===
--- clang/test/CodeGen/X86/x86_64-PR42672.c
+++ clang/test/CodeGen/X86/x86_64-PR42672.c
@@ -5,6 +5,7 @@
 // RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -DPOSSIBLE_X -emit-llvm %s 
-o - 2>&1 | FileCheck %s --check-prefix=CHECK-X
 // RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_X 
-emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_X
 // RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_9BYTES 
-emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_9BYTES
+// RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_9BYTES_V2 
-emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_9BYTES_V2

 // Make sure Clang doesn't treat |lockval| as asm input.
 void _raw_spin_lock(void) {
@@ -115,3 +116,12 @@
 }

 // CHECK-IMPOSSIBLE_9BYTES: impossible constraint in asm: can't store value 
into a register
+
+void crbug_999160_regtest_v2(void) {
+#ifdef IMPOSSIBLE_9BYTES_V2
+  char buf[9];
+  asm(""
+  : "=r"(buf) : "0"(buf));
+#endif
+}
+// CHECK-IMPOSSIBLE_9BYTES_V2: impossible constraint in asm: can't store value 
into a register
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -2513,10 +2513,8 @@
   Arg = Builder.CreateZExt(Arg, OutputTy);
 else if (isa(OutputTy))
   Arg = Builder.CreateZExt(Arg, IntPtrTy);
-else {
-  assert(OutputTy->isFloatingPointTy() && "Unexpected output type");
+else if (OutputTy->isFloatingPointTy())
   Arg = Builder.CreateFPExt(Arg, OutputTy);
-}
   }
   // Deal with the tied operands' constraint code in adjustInlineAsmType.
   ReplaceConstraint = OutputConstraints[Output];


Index: clang/test/CodeGen/X86/x86_64-PR42672.c
===
--- clang/test/CodeGen/X86/x86_64-PR42672.c
+++ clang/test/CodeGen/X86/x86_64-PR42672.c
@@ -5,6 +5,7 @@
 // RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -DPOSSIBLE_X -emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-X
 // RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_X -emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_X
 // RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_9BYTES -emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_9BYTES
+// RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_9BYTES_V2 -emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_9BYTES_V2

 // Make sure Clang doesn't treat |lockval| as asm input.
 void _raw_spin_lock(void) {
@@ -115,3 +116,12 @@
 }

 // CHECK-IMPOSSIBLE_9BYTES: impossible constraint in asm: can't store value into a register
+
+void crbug_999160_regtest_v2(void) {
+#ifdef IMPOSSIBLE_9BYTES_V2
+  char buf[9];
+  asm(""
+  : "=r"(buf) : "0"(buf));
+#endif
+}
+// CHECK-IMPOSSIBLE_9BYTES_V2: impossible constraint in asm: can't store value into a register
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -2513,10 +2513,8 @@
   Arg = Builder.CreateZExt(Arg, OutputTy);
 else if (isa(OutputTy))
   Arg = Builder.CreateZExt(Arg, IntPtrTy);
-else {
-  assert(OutputTy->isFloatingPointTy() && "Unexpected output type");
+else if (OutputTy->isFloatingPointTy())
   Arg = Builder.CreateFPExt(Arg, OutputTy);
-}
   }
   // Deal with the tied operands' constraint code in adjustInlineAsmType.
   ReplaceConstraint = OutputConstraints[Output];
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D120596: [clang][CGStmt] fix crash on invalid asm statement

2022-02-25 Thread Tong Zhang via Phabricator via cfe-commits 
ztong0001 updated this revision to Diff 411577.
ztong0001 added a comment.

reformat code


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120596/new/

https://reviews.llvm.org/D120596

Files:
  clang/lib/CodeGen/CGStmt.cpp
  clang/test/CodeGen/X86/x86_64-PR42672.c


Index: clang/test/CodeGen/X86/x86_64-PR42672.c
===
--- clang/test/CodeGen/X86/x86_64-PR42672.c
+++ clang/test/CodeGen/X86/x86_64-PR42672.c
@@ -5,6 +5,7 @@
 // RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -DPOSSIBLE_X -emit-llvm %s 
-o - 2>&1 | FileCheck %s --check-prefix=CHECK-X
 // RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_X 
-emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_X
 // RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_9BYTES 
-emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_9BYTES
+// RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_9BYTES_V2 
-emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_9BYTES_V2

 // Make sure Clang doesn't treat |lockval| as asm input.
 void _raw_spin_lock(void) {
@@ -115,3 +116,13 @@
 }

 // CHECK-IMPOSSIBLE_9BYTES: impossible constraint in asm: can't store value 
into a register
+
+void crbug_999160_regtest_v2(void) {
+#ifdef IMPOSSIBLE_9BYTES_V2
+  char buf[9];
+  asm(""
+  : "=r"(buf)
+  : "0"(buf));
+#endif
+}
+// CHECK-IMPOSSIBLE_9BYTES_V2: impossible constraint in asm: can't store value 
into a register
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -2513,10 +2513,8 @@
   Arg = Builder.CreateZExt(Arg, OutputTy);
 else if (isa(OutputTy))
   Arg = Builder.CreateZExt(Arg, IntPtrTy);
-else {
-  assert(OutputTy->isFloatingPointTy() && "Unexpected output type");
+else if (OutputTy->isFloatingPointTy())
   Arg = Builder.CreateFPExt(Arg, OutputTy);
-}
   }
   // Deal with the tied operands' constraint code in adjustInlineAsmType.
   ReplaceConstraint = OutputConstraints[Output];


Index: clang/test/CodeGen/X86/x86_64-PR42672.c
===
--- clang/test/CodeGen/X86/x86_64-PR42672.c
+++ clang/test/CodeGen/X86/x86_64-PR42672.c
@@ -5,6 +5,7 @@
 // RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -DPOSSIBLE_X -emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-X
 // RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_X -emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_X
 // RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_9BYTES -emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_9BYTES
+// RUN: not %clang_cc1 -triple=x86_64-unknown-linux-gnu -DIMPOSSIBLE_9BYTES_V2 -emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=CHECK-IMPOSSIBLE_9BYTES_V2

 // Make sure Clang doesn't treat |lockval| as asm input.
 void _raw_spin_lock(void) {
@@ -115,3 +116,13 @@
 }

 // CHECK-IMPOSSIBLE_9BYTES: impossible constraint in asm: can't store value into a register
+
+void crbug_999160_regtest_v2(void) {
+#ifdef IMPOSSIBLE_9BYTES_V2
+  char buf[9];
+  asm(""
+  : "=r"(buf)
+  : "0"(buf));
+#endif
+}
+// CHECK-IMPOSSIBLE_9BYTES_V2: impossible constraint in asm: can't store value into a register
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -2513,10 +2513,8 @@
   Arg = Builder.CreateZExt(Arg, OutputTy);
 else if (isa(OutputTy))
   Arg = Builder.CreateZExt(Arg, IntPtrTy);
-else {
-  assert(OutputTy->isFloatingPointTy() && "Unexpected output type");
+else if (OutputTy->isFloatingPointTy())
   Arg = Builder.CreateFPExt(Arg, OutputTy);
-}
   }
   // Deal with the tied operands' constraint code in adjustInlineAsmType.
   ReplaceConstraint = OutputConstraints[Output];
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D119816: [SanitizerBounds] Add support for NoSanitizeBounds function

2022-02-28 Thread Tong Zhang via Phabricator via cfe-commits 
ztong0001 added a comment.

Hi Marco,
Yes I need help landing it.
Please use Tong Zhang
Thanks,

- Tong


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119816/new/

https://reviews.llvm.org/D119816

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D119816: [SanitizerBounds] Add support for NoSanitizeBounds function

2022-03-01 Thread Tong Zhang via Phabricator via cfe-commits 
ztong0001 added a comment.

Hi Marco,
@melver, Could you please help me landing it? I don't have write permission to 
the repo.
Please use Tong Zhang
Thanks,

Tong


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119816/new/

https://reviews.llvm.org/D119816

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D120596: [clang][CGStmt] fix crash on invalid asm statement

2022-03-02 Thread Tong Zhang via Phabricator via cfe-commits 
ztong0001 added inline comments.



Comment at: clang/lib/CodeGen/CGStmt.cpp:2517
+else if (OutputTy->isFloatingPointTy())
   Arg = Builder.CreateFPExt(Arg, OutputTy);
   }

MaskRay wrote:
> Shall we keep the assert (in a new `else` branch) just in case there are 
> other cases which cannot be handled?
IMO adding assert() in else branch still has the same issue with the inline asm 
in description.

This portion of the code is to extend argument to a longer type, if it cannot 
do so the patch will simply skip and let it fall back to old behavior and print 
out 

`impossible constraint in asm: can't store value into a register`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120596/new/

https://reviews.llvm.org/D120596

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D120596: [clang][CGStmt] fix crash on invalid asm statement

2022-03-02 Thread Tong Zhang via Phabricator via cfe-commits 
ztong0001 added a comment.

Hi Fangrui @MaskRay,
Thanks for your review. Could you please also help me landing it since I don't 
have write permission to the repo.
Please use Tong Zhang
Thanks and have a good one!

- Tong


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120596/new/

https://reviews.llvm.org/D120596

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits