Re: [PATCH] openmp: fix UBSAN error at gcc/fortran/openmp.c:4737
On 8/17/20 11:15 AM, Martin Liška wrote: I'm suggesting one more clean up that uses static assert instead of a run-time check. I concur that compile-time checks are nicer. LGTM – it should be able catch this kind of mistakes. Tobias Thoughts? Martin 0001-opnemp-add-static-assert-for-clause_names.patch From c9aee2c44d5cf7e417d381988b2f4900e9ea8b05 Mon Sep 17 00:00:00 2001 From: Martin Liska Date: Mon, 17 Aug 2020 11:14:13 +0200 Subject: [PATCH] opnemp: add static assert for clause_names. gcc/fortran/ChangeLog: * openmp.c (resolve_omp_clauses): Add static assert for OMP_LIST_NUM and size of clause_names array. Remove check that is always true. --- gcc/fortran/openmp.c | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/gcc/fortran/openmp.c b/gcc/fortran/openmp.c index 60d8e5573c2..4d33a450a33 100644 --- a/gcc/fortran/openmp.c +++ b/gcc/fortran/openmp.c @@ -4371,6 +4371,7 @@ resolve_omp_clauses (gfc_code *code, gfc_omp_clauses *omp_clauses, "TO", "FROM", "REDUCTION", "DEVICE_RESIDENT", "LINK", "USE_DEVICE", "CACHE", "IS_DEVICE_PTR", "USE_DEVICE_PTR", "USE_DEVICE_ADDR", "NONTEMPORAL" }; + STATIC_ASSERT (ARRAY_SIZE (clause_names) == OMP_LIST_NUM); if (omp_clauses == NULL) return; @@ -4732,12 +4733,7 @@ resolve_omp_clauses (gfc_code *code, gfc_omp_clauses *omp_clauses, for (list = 0; list < OMP_LIST_NUM; list++) if ((n = omp_clauses->lists[list]) != NULL) { - const char *name; - - if (list < OMP_LIST_NUM) - name = clause_names[list]; - else - gcc_unreachable (); + const char *name = clause_names[list]; switch (list) { -- 2.28.0 - Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander Walter
Re: [PATCH] openmp: fix UBSAN error at gcc/fortran/openmp.c:4737
On 8/17/20 10:52 AM, Tobias Burnus wrote: LGTM & thanks! – Sorry for missing it. That happens. (I re-checked against the OMP_LIST_* enum and it seems to be only missing one.) Good. I'm suggesting one more clean up that uses static assert instead of a run-time check. Thoughts? Martin >From c9aee2c44d5cf7e417d381988b2f4900e9ea8b05 Mon Sep 17 00:00:00 2001 From: Martin Liska Date: Mon, 17 Aug 2020 11:14:13 +0200 Subject: [PATCH] opnemp: add static assert for clause_names. gcc/fortran/ChangeLog: * openmp.c (resolve_omp_clauses): Add static assert for OMP_LIST_NUM and size of clause_names array. Remove check that is always true. --- gcc/fortran/openmp.c | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/gcc/fortran/openmp.c b/gcc/fortran/openmp.c index 60d8e5573c2..4d33a450a33 100644 --- a/gcc/fortran/openmp.c +++ b/gcc/fortran/openmp.c @@ -4371,6 +4371,7 @@ resolve_omp_clauses (gfc_code *code, gfc_omp_clauses *omp_clauses, "TO", "FROM", "REDUCTION", "DEVICE_RESIDENT", "LINK", "USE_DEVICE", "CACHE", "IS_DEVICE_PTR", "USE_DEVICE_PTR", "USE_DEVICE_ADDR", "NONTEMPORAL" }; + STATIC_ASSERT (ARRAY_SIZE (clause_names) == OMP_LIST_NUM); if (omp_clauses == NULL) return; @@ -4732,12 +4733,7 @@ resolve_omp_clauses (gfc_code *code, gfc_omp_clauses *omp_clauses, for (list = 0; list < OMP_LIST_NUM; list++) if ((n = omp_clauses->lists[list]) != NULL) { - const char *name; - - if (list < OMP_LIST_NUM) - name = clause_names[list]; - else - gcc_unreachable (); + const char *name = clause_names[list]; switch (list) { -- 2.28.0
Re: [PATCH] openmp: fix UBSAN error at gcc/fortran/openmp.c:4737
On 8/17/20 10:41 AM, Martin Liška wrote: Since 21cfe724cbdc30612bf1ef59b26f19ada2210832 there's a new OMP_LIST_NONTEMPORAL value, but it was missing in resolve_omp_clauses static array that is defined at the function beginning: gcc/fortran/ChangeLog: * openmp.c (resolve_omp_clauses): Add NONTEMPORAL to clause names. LGTM & thanks! – Sorry for missing it. (I re-checked against the OMP_LIST_* enum and it seems to be only missing one.) Tobias --- gcc/fortran/openmp.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/gcc/fortran/openmp.c b/gcc/fortran/openmp.c index c44a2530b88..60d8e5573c2 100644 --- a/gcc/fortran/openmp.c +++ b/gcc/fortran/openmp.c @@ -4369,7 +4369,8 @@ resolve_omp_clauses (gfc_code *code, gfc_omp_clauses *omp_clauses, = { "PRIVATE", "FIRSTPRIVATE", "LASTPRIVATE", "COPYPRIVATE", "SHARED", "COPYIN", "UNIFORM", "ALIGNED", "LINEAR", "DEPEND", "MAP", "TO", "FROM", "REDUCTION", "DEVICE_RESIDENT", "LINK", "USE_DEVICE", -"CACHE", "IS_DEVICE_PTR", "USE_DEVICE_PTR", "USE_DEVICE_ADDR" }; +"CACHE", "IS_DEVICE_PTR", "USE_DEVICE_PTR", "USE_DEVICE_ADDR", +"NONTEMPORAL" }; if (omp_clauses == NULL) return; - Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander Walter