[PATCH] D31591: Fix a bug which access nullptr and cause segmentation fault
v.g.vassilev added a comment. Landed in r300313. https://reviews.llvm.org/D31591 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31591: Fix a bug which access nullptr and cause segmentation fault
yamaguchi added a comment. I don't have a commit access. Can you commit? https://reviews.llvm.org/D31591 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31591: Fix a bug which access nullptr and cause segmentation fault
v.g.vassilev accepted this revision. v.g.vassilev added a comment. LGTM. https://reviews.llvm.org/D31591 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D31591: Fix a bug which access nullptr and cause segmentation fault
It seems to me the test have already been simplified enough, so I'm not
quite sure what you are trying to do more than this. Isn't this just ready
to submit?
On Wed, Apr 12, 2017 at 5:46 PM, Yuka Takahashi via Phabricator <
revi...@reviews.llvm.org> wrote:
> yamaguchi updated this revision to Diff 95064.
> yamaguchi added a comment.
>
> I've been trying to minimal the testcase, add comments to describe what it
> is testing, and fix styles of the testcase properly.
> However, I don't have clear idea what will be the best. I would like to
> ask for the advice.
>
>
> https://reviews.llvm.org/D31591
>
> Files:
> lib/Sema/SemaInit.cpp
> test/Sema/designated-initializers.c
>
>
> Index: lib/Sema/SemaInit.cpp
> ===
> --- lib/Sema/SemaInit.cpp
> +++ lib/Sema/SemaInit.cpp
> @@ -2269,15 +2269,17 @@
>assert(StructuredList->getNumInits() == 1
> && "A union should never have more than one
> initializer!");
>
> - // We're about to throw away an initializer, emit warning.
> - SemaRef.Diag(D->getFieldLoc(),
> - diag::warn_initializer_overrides)
> -<< D->getSourceRange();
>Expr *ExistingInit = StructuredList->getInit(0);
> - SemaRef.Diag(ExistingInit->getLocStart(),
> - diag::note_previous_initializer)
> -<< /*FIXME:has side effects=*/0
> -<< ExistingInit->getSourceRange();
> + if (ExistingInit) {
> +// We're about to throw away an initializer, emit warning.
> +SemaRef.Diag(D->getFieldLoc(),
> + diag::warn_initializer_overrides)
> + << D->getSourceRange();
> +SemaRef.Diag(ExistingInit->getLocStart(),
> + diag::note_previous_initializer)
> + << /*FIXME:has side effects=*/0
> + << ExistingInit->getSourceRange();
> + }
>
>// remove existing initializer
>StructuredList->resizeInits(SemaRef.Context, 0);
> Index: test/Sema/designated-initializers.c
> ===
> --- test/Sema/designated-initializers.c
> +++ test/Sema/designated-initializers.c
> @@ -351,3 +351,20 @@
>{ { 'f', 'o', 'o' }, 1 },
>[0].L[4] = 'x' // no-warning
> };
> +
> +struct {
> + struct { } s1;
> + union {
> +int a;
> +int b;
> + } u1;
> +} s = {
> + .s1 = {
> +.x = 0, // expected-error{{field designator}}
> + },
> +
> + .u1 = {
> +.a = 0,
> +.b = 0,
> + },
> +};
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31591: Fix a bug which access nullptr and cause segmentation fault
yamaguchi updated this revision to Diff 95064.
yamaguchi added a comment.
I've been trying to minimal the testcase, add comments to describe what it is
testing, and fix styles of the testcase properly.
However, I don't have clear idea what will be the best. I would like to ask for
the advice.
https://reviews.llvm.org/D31591
Files:
lib/Sema/SemaInit.cpp
test/Sema/designated-initializers.c
Index: lib/Sema/SemaInit.cpp
===
--- lib/Sema/SemaInit.cpp
+++ lib/Sema/SemaInit.cpp
@@ -2269,15 +2269,17 @@
assert(StructuredList->getNumInits() == 1
&& "A union should never have more than one initializer!");
- // We're about to throw away an initializer, emit warning.
- SemaRef.Diag(D->getFieldLoc(),
- diag::warn_initializer_overrides)
-<< D->getSourceRange();
Expr *ExistingInit = StructuredList->getInit(0);
- SemaRef.Diag(ExistingInit->getLocStart(),
- diag::note_previous_initializer)
-<< /*FIXME:has side effects=*/0
-<< ExistingInit->getSourceRange();
+ if (ExistingInit) {
+// We're about to throw away an initializer, emit warning.
+SemaRef.Diag(D->getFieldLoc(),
+ diag::warn_initializer_overrides)
+ << D->getSourceRange();
+SemaRef.Diag(ExistingInit->getLocStart(),
+ diag::note_previous_initializer)
+ << /*FIXME:has side effects=*/0
+ << ExistingInit->getSourceRange();
+ }
// remove existing initializer
StructuredList->resizeInits(SemaRef.Context, 0);
Index: test/Sema/designated-initializers.c
===
--- test/Sema/designated-initializers.c
+++ test/Sema/designated-initializers.c
@@ -351,3 +351,20 @@
{ { 'f', 'o', 'o' }, 1 },
[0].L[4] = 'x' // no-warning
};
+
+struct {
+ struct { } s1;
+ union {
+int a;
+int b;
+ } u1;
+} s = {
+ .s1 = {
+.x = 0, // expected-error{{field designator}}
+ },
+
+ .u1 = {
+.a = 0,
+.b = 0,
+ },
+};
Index: lib/Sema/SemaInit.cpp
===
--- lib/Sema/SemaInit.cpp
+++ lib/Sema/SemaInit.cpp
@@ -2269,15 +2269,17 @@
assert(StructuredList->getNumInits() == 1
&& "A union should never have more than one initializer!");
- // We're about to throw away an initializer, emit warning.
- SemaRef.Diag(D->getFieldLoc(),
- diag::warn_initializer_overrides)
-<< D->getSourceRange();
Expr *ExistingInit = StructuredList->getInit(0);
- SemaRef.Diag(ExistingInit->getLocStart(),
- diag::note_previous_initializer)
-<< /*FIXME:has side effects=*/0
-<< ExistingInit->getSourceRange();
+ if (ExistingInit) {
+// We're about to throw away an initializer, emit warning.
+SemaRef.Diag(D->getFieldLoc(),
+ diag::warn_initializer_overrides)
+ << D->getSourceRange();
+SemaRef.Diag(ExistingInit->getLocStart(),
+ diag::note_previous_initializer)
+ << /*FIXME:has side effects=*/0
+ << ExistingInit->getSourceRange();
+ }
// remove existing initializer
StructuredList->resizeInits(SemaRef.Context, 0);
Index: test/Sema/designated-initializers.c
===
--- test/Sema/designated-initializers.c
+++ test/Sema/designated-initializers.c
@@ -351,3 +351,20 @@
{ { 'f', 'o', 'o' }, 1 },
[0].L[4] = 'x' // no-warning
};
+
+struct {
+ struct { } s1;
+ union {
+int a;
+int b;
+ } u1;
+} s = {
+ .s1 = {
+.x = 0, // expected-error{{field designator}}
+ },
+
+ .u1 = {
+.a = 0,
+.b = 0,
+ },
+};
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31591: Fix a bug which access nullptr and cause segmentation fault
v.g.vassilev added a comment. ping... https://reviews.llvm.org/D31591 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31591: Fix a bug which access nullptr and cause segmentation fault
v.g.vassilev added a comment. @yamaguchi, I'd support Akira's comment. Could you place the minimal test example in the suggested file? https://reviews.llvm.org/D31591 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31591: Fix a bug which access nullptr and cause segmentation fault
ruiu added a comment. Is this a minimal test case that can produce the issue? It'd be awesome if you can reduce it. sema-segvcheck.c is not a good name for this test because that name can be used for any crash bug. You want to see other files in the same directory to name your file so that it's consistent with other files. If you don't come up with a name, I'd name it pr32280.c. https://reviews.llvm.org/D31591 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31591: Fix a bug which access nullptr and cause segmentation fault
ahatanak added inline comments.
Comment at: clang/test/Sema/sema-segvcheck.c:3
+// RUN: %clang_cc1 -fsyntax-only %s; test $? -eq 1
+
+typedef struct {
You can simplify the test case. Compiling the following code still segfaults:
```
typedef struct {
unsigned long long house;
} struct_0;
typedef union {
unsigned cows;
unsigned char c;
} union_1;
typedef struct {
struct_0 s0;
union_1 s1;
} struct_2;
struct_2 s = {
.s0 = {
.dog = 0x0009,
},
.s1 = {
.cows = 0x0055,
.c = 1,
},
};
```
Also, would it be better to add this test to an existing file (e.g.,
test/Sema/designated-initializers.c) rather than creating a new file?
https://reviews.llvm.org/D31591
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31591: Fix a bug which access nullptr and cause segmentation fault
yamaguchi updated this revision to Diff 94090.
yamaguchi added a comment.
Add relative path to the file.
https://reviews.llvm.org/D31591
Files:
clang/test/Sema/sema-segvcheck.c
lib/Sema/SemaInit.cpp
Index: clang/test/Sema/sema-segvcheck.c
===
--- clang/test/Sema/sema-segvcheck.c
+++ clang/test/Sema/sema-segvcheck.c
@@ -0,0 +1,50 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only %s; test $? -eq 1
+
+typedef struct {
+ union {
+unsigned long long house;
+struct {
+ unsigned cat1;
+ unsigned cat2;
+};
+ };
+} struct_0;
+
+
+typedef struct {
+ union {
+struct {
+ union {
+unsigned cows;
+struct {
+ unsigned char c:1;
+};
+ };
+};
+ };
+
+ union {
+struct {
+ unsigned bird0;
+ unsigned bird1;
+};
+ };
+} struct_1;
+
+
+typedef struct {
+ struct_0 s0;
+ struct_1 s1[1];
+} struct_2;
+
+struct_2 s = {
+ .s0 = {
+.dog = 0x, // expected-error{{field designator}}
+ },
+
+ .s1[0] = {
+.cows = 0x5050,
+.c = 1,
+ },
+};
Index: lib/Sema/SemaInit.cpp
===
--- lib/Sema/SemaInit.cpp
+++ lib/Sema/SemaInit.cpp
@@ -2269,15 +2269,17 @@
assert(StructuredList->getNumInits() == 1
&& "A union should never have more than one initializer!");
- // We're about to throw away an initializer, emit warning.
- SemaRef.Diag(D->getFieldLoc(),
- diag::warn_initializer_overrides)
-<< D->getSourceRange();
Expr *ExistingInit = StructuredList->getInit(0);
- SemaRef.Diag(ExistingInit->getLocStart(),
- diag::note_previous_initializer)
-<< /*FIXME:has side effects=*/0
-<< ExistingInit->getSourceRange();
+ if (ExistingInit) {
+// We're about to throw away an initializer, emit warning.
+SemaRef.Diag(D->getFieldLoc(),
+ diag::warn_initializer_overrides)
+ << D->getSourceRange();
+SemaRef.Diag(ExistingInit->getLocStart(),
+ diag::note_previous_initializer)
+ << /*FIXME:has side effects=*/0
+ << ExistingInit->getSourceRange();
+ }
// remove existing initializer
StructuredList->resizeInits(SemaRef.Context, 0);
Index: clang/test/Sema/sema-segvcheck.c
===
--- clang/test/Sema/sema-segvcheck.c
+++ clang/test/Sema/sema-segvcheck.c
@@ -0,0 +1,50 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only %s; test $? -eq 1
+
+typedef struct {
+ union {
+unsigned long long house;
+struct {
+ unsigned cat1;
+ unsigned cat2;
+};
+ };
+} struct_0;
+
+
+typedef struct {
+ union {
+struct {
+ union {
+unsigned cows;
+struct {
+ unsigned char c:1;
+};
+ };
+};
+ };
+
+ union {
+struct {
+ unsigned bird0;
+ unsigned bird1;
+};
+ };
+} struct_1;
+
+
+typedef struct {
+ struct_0 s0;
+ struct_1 s1[1];
+} struct_2;
+
+struct_2 s = {
+ .s0 = {
+.dog = 0x, // expected-error{{field designator}}
+ },
+
+ .s1[0] = {
+.cows = 0x5050,
+.c = 1,
+ },
+};
Index: lib/Sema/SemaInit.cpp
===
--- lib/Sema/SemaInit.cpp
+++ lib/Sema/SemaInit.cpp
@@ -2269,15 +2269,17 @@
assert(StructuredList->getNumInits() == 1
&& "A union should never have more than one initializer!");
- // We're about to throw away an initializer, emit warning.
- SemaRef.Diag(D->getFieldLoc(),
- diag::warn_initializer_overrides)
-<< D->getSourceRange();
Expr *ExistingInit = StructuredList->getInit(0);
- SemaRef.Diag(ExistingInit->getLocStart(),
- diag::note_previous_initializer)
-<< /*FIXME:has side effects=*/0
-<< ExistingInit->getSourceRange();
+ if (ExistingInit) {
+// We're about to throw away an initializer, emit warning.
+SemaRef.Diag(D->getFieldLoc(),
+ diag::warn_initializer_overrides)
+ << D->getSourceRange();
+SemaRef.Diag(ExistingInit->getLocStart(),
+ diag::note_previous_initializer)
+ << /*FIXME:has side effects=*/0
+ << ExistingInit->getSourceRange();
+ }
// remove existing initializer
StructuredList->resizeInits(SemaRef.Context, 0);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31591: Fix a bug which access nullptr and cause segmentation fault
yamaguchi updated this revision to Diff 93864.
yamaguchi added a comment.
Made unified diff for the testcase and SemaInit.cpp.
https://reviews.llvm.org/D31591
Files:
SemaInit.cpp
sema-segvcheck.c
Index: sema-segvcheck.c
===
--- sema-segvcheck.c
+++ sema-segvcheck.c
@@ -0,0 +1,50 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only %s; test $? -eq 1
+
+typedef struct {
+ union {
+unsigned long long house;
+struct {
+ unsigned cat1;
+ unsigned cat2;
+};
+ };
+} struct_0;
+
+
+typedef struct {
+ union {
+struct {
+ union {
+unsigned cows;
+struct {
+ unsigned char c:1;
+};
+ };
+};
+ };
+
+ union {
+struct {
+ unsigned bird0;
+ unsigned bird1;
+};
+ };
+} struct_1;
+
+
+typedef struct {
+ struct_0 s0;
+ struct_1 s1[1];
+} struct_2;
+
+struct_2 s = {
+ .s0 = {
+.dog = 0x, // expected-error{{field designator}}
+ },
+
+ .s1[0] = {
+.cows = 0x5050,
+.c = 1,
+ },
+};
Index: SemaInit.cpp
===
--- SemaInit.cpp
+++ SemaInit.cpp
@@ -2260,15 +2260,17 @@
assert(StructuredList->getNumInits() == 1
&& "A union should never have more than one initializer!");
- // We're about to throw away an initializer, emit warning.
- SemaRef.Diag(D->getFieldLoc(),
- diag::warn_initializer_overrides)
-<< D->getSourceRange();
Expr *ExistingInit = StructuredList->getInit(0);
- SemaRef.Diag(ExistingInit->getLocStart(),
- diag::note_previous_initializer)
-<< /*FIXME:has side effects=*/0
-<< ExistingInit->getSourceRange();
+ if (ExistingInit) {
+// We're about to throw away an initializer, emit warning.
+SemaRef.Diag(D->getFieldLoc(),
+ diag::warn_initializer_overrides)
+ << D->getSourceRange();
+SemaRef.Diag(ExistingInit->getLocStart(),
+ diag::note_previous_initializer)
+ << /*FIXME:has side effects=*/0
+ << ExistingInit->getSourceRange();
+ }
// remove existing initializer
StructuredList->resizeInits(SemaRef.Context, 0);
Index: sema-segvcheck.c
===
--- sema-segvcheck.c
+++ sema-segvcheck.c
@@ -0,0 +1,50 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only %s; test $? -eq 1
+
+typedef struct {
+ union {
+unsigned long long house;
+struct {
+ unsigned cat1;
+ unsigned cat2;
+};
+ };
+} struct_0;
+
+
+typedef struct {
+ union {
+struct {
+ union {
+unsigned cows;
+struct {
+ unsigned char c:1;
+};
+ };
+};
+ };
+
+ union {
+struct {
+ unsigned bird0;
+ unsigned bird1;
+};
+ };
+} struct_1;
+
+
+typedef struct {
+ struct_0 s0;
+ struct_1 s1[1];
+} struct_2;
+
+struct_2 s = {
+ .s0 = {
+.dog = 0x, // expected-error{{field designator}}
+ },
+
+ .s1[0] = {
+.cows = 0x5050,
+.c = 1,
+ },
+};
Index: SemaInit.cpp
===
--- SemaInit.cpp
+++ SemaInit.cpp
@@ -2260,15 +2260,17 @@
assert(StructuredList->getNumInits() == 1
&& "A union should never have more than one initializer!");
- // We're about to throw away an initializer, emit warning.
- SemaRef.Diag(D->getFieldLoc(),
- diag::warn_initializer_overrides)
-<< D->getSourceRange();
Expr *ExistingInit = StructuredList->getInit(0);
- SemaRef.Diag(ExistingInit->getLocStart(),
- diag::note_previous_initializer)
-<< /*FIXME:has side effects=*/0
-<< ExistingInit->getSourceRange();
+ if (ExistingInit) {
+// We're about to throw away an initializer, emit warning.
+SemaRef.Diag(D->getFieldLoc(),
+ diag::warn_initializer_overrides)
+ << D->getSourceRange();
+SemaRef.Diag(ExistingInit->getLocStart(),
+ diag::note_previous_initializer)
+ << /*FIXME:has side effects=*/0
+ << ExistingInit->getSourceRange();
+ }
// remove existing initializer
StructuredList->resizeInits(SemaRef.Context, 0);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31591: Fix a bug which access nullptr and cause segmentation fault
yamaguchi updated this revision to Diff 93860.
yamaguchi added a comment.
Moved comment inside if (ExistingInit).
https://reviews.llvm.org/D31591
Files:
SemaInit.cpp
Index: SemaInit.cpp
===
--- SemaInit.cpp
+++ SemaInit.cpp
@@ -2260,15 +2260,17 @@
assert(StructuredList->getNumInits() == 1
&& "A union should never have more than one initializer!");
- // We're about to throw away an initializer, emit warning.
- SemaRef.Diag(D->getFieldLoc(),
- diag::warn_initializer_overrides)
-<< D->getSourceRange();
Expr *ExistingInit = StructuredList->getInit(0);
- SemaRef.Diag(ExistingInit->getLocStart(),
- diag::note_previous_initializer)
-<< /*FIXME:has side effects=*/0
-<< ExistingInit->getSourceRange();
+ if (ExistingInit) {
+// We're about to throw away an initializer, emit warning.
+SemaRef.Diag(D->getFieldLoc(),
+ diag::warn_initializer_overrides)
+ << D->getSourceRange();
+SemaRef.Diag(ExistingInit->getLocStart(),
+ diag::note_previous_initializer)
+ << /*FIXME:has side effects=*/0
+ << ExistingInit->getSourceRange();
+ }
// remove existing initializer
StructuredList->resizeInits(SemaRef.Context, 0);
Index: SemaInit.cpp
===
--- SemaInit.cpp
+++ SemaInit.cpp
@@ -2260,15 +2260,17 @@
assert(StructuredList->getNumInits() == 1
&& "A union should never have more than one initializer!");
- // We're about to throw away an initializer, emit warning.
- SemaRef.Diag(D->getFieldLoc(),
- diag::warn_initializer_overrides)
-<< D->getSourceRange();
Expr *ExistingInit = StructuredList->getInit(0);
- SemaRef.Diag(ExistingInit->getLocStart(),
- diag::note_previous_initializer)
-<< /*FIXME:has side effects=*/0
-<< ExistingInit->getSourceRange();
+ if (ExistingInit) {
+// We're about to throw away an initializer, emit warning.
+SemaRef.Diag(D->getFieldLoc(),
+ diag::warn_initializer_overrides)
+ << D->getSourceRange();
+SemaRef.Diag(ExistingInit->getLocStart(),
+ diag::note_previous_initializer)
+ << /*FIXME:has side effects=*/0
+ << ExistingInit->getSourceRange();
+ }
// remove existing initializer
StructuredList->resizeInits(SemaRef.Context, 0);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
