blob: 495b3bc5efc106785ea187cacf7ee9d2f867018a [file] [log] [blame]
Kevin Pengef0aa6c2023-10-08 14:33:31 +08001From 1b199ce317d02c46770269631fea644cefb63978 Mon Sep 17 00:00:00 2001
2From: Laurence Lundblade <laurencelundblade@users.noreply.github.com>
3Date: Sun, 12 Feb 2023 13:21:44 -0800
4Subject: [PATCH] Disable gcc -Wmaybe-uninitialized because of false positives
5 (#179)
6
7For some versions of gcc (but not all) -Wall enables -Wmaybe-uninitialized.
8
9While there are no uninitialized variables in QCBOR, warnings are still produced with some versions of gcc and with some optimization options for gcc. This just disables the warning entirely for qcbor_decode.c and notes where the warnings were checked.
10
11It is well understood that -Wmaybe-uninitialized produces false positives.
12
13QCBOR has been through full and proper static analysis so we know it doesn't have uninitialized variables. The cases that -Wmaybe-uninitialized complained about in one case (I can't reproduce them) have been checked and noted.
14
15Note that just slamming initialization on to the variables to prevent the warnings is actually a bit dangerous because you don't know what the proper initialization value should be. You actually
16have to read and understand the code to initialize correctly. That or confirm there is no
17uninitialized variables. Both have been done in this case.
18
19
20
21* Disable -Wmaybe-uninitialized because of false positives
22
23* Improved explanatory text
24
25---------
26
27Co-authored-by: Laurence Lundblade <lgl@securitytheory.com>
28---
29 src/qcbor_decode.c | 58 ++++++++++++++++++++++++++++++++++++++++++----
30 1 file changed, 53 insertions(+), 5 deletions(-)
31
32diff --git a/src/qcbor_decode.c b/src/qcbor_decode.c
33index 8a547ee..57aab97 100644
34--- a/src/qcbor_decode.c
35+++ b/src/qcbor_decode.c
36@@ -1,6 +1,6 @@
37 /*==============================================================================
38 Copyright (c) 2016-2018, The Linux Foundation.
39- Copyright (c) 2018-2022, Laurence Lundblade.
40+ Copyright (c) 2018-2023, Laurence Lundblade.
41 Copyright (c) 2021, Arm Limited.
42 All rights reserved.
43
44@@ -46,6 +46,54 @@ IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
45 #endif /* QCBOR_DISABLE_FLOAT_HW_USE */
46
47
48+#if (defined(__GNUC__) && !defined(__clang__))
49+/*
50+ * This is how the -Wmaybe-uninitialized compiler warning is
51+ * handled. It can’t be ignored because some version of gcc enable it
52+ * with -Wall which is a common and useful gcc warning option. It also
53+ * can’t be ignored because it is the goal of QCBOR to compile clean
54+ * out of the box in all environments.
55+ *
56+ * The big problem with -Wmaybe-uninitialized is that it generates
57+ * false positives. It complains things are uninitialized when they
58+ * are not. This is because it is not a thorough static analyzer. This
59+ * is why “maybe” is in its name. The problem is it is just not
60+ * thorough enough to understand all the code (and someone saw fit to
61+ * put it in gcc and worse to enable it with -Wall).
62+ *
63+ * One solution would be to change the code so -Wmaybe-uninitialized
64+ * doesn’t get confused, for example adding an unnecessary extra
65+ * initialization to zero. (If variables were truly uninitialized, the
66+ * correct path is to understand the code thoroughly and set them to
67+ * the correct value at the correct time; in essence this is already
68+ * done; -Wmaybe-uninitialized just can’t tell). This path is not
69+ * taken because it makes the code bigger and is kind of the tail
70+ * wagging the dog.
71+ *
72+ * The solution here is to just use a pragma to disable it for the
73+ * whole file. Disabling it for each line makes the code fairly ugly
74+ * requiring #pragma to push, pop and ignore. Another reason is the
75+ * warnings issues vary by version of gcc and which optimization
76+ * optimizations are selected. Another reason is that compilers other
77+ * than gcc don’t have -Wmaybe-uninitialized.
78+ *
79+ * One may ask how to be sure these warnings are false positives and
80+ * not real issues. 1) The code has been read carefully to check. 2)
81+ * Testing is pretty thorough. 3) This code has been run through
82+ * thorough high-quality static analyzers.
83+ *
84+ * In particularly, most of the warnings are about
85+ * Item.Item->uDataType being uninitialized. QCBORDecode_GetNext()
86+ * *always* sets this value and test case confirm
87+ * this. -Wmaybe-uninitialized just can't tell.
88+ *
89+ * https://stackoverflow.com/questions/5080848/disable-gcc-may-be-used-uninitialized-on-a-particular-variable
90+ */
91+#pragma GCC diagnostic ignored "-Wmaybe-uninitialized"
92+#endif
93+
94+
95+
96
97 #define SIZEOF_C_ARRAY(array,type) (sizeof(array)/sizeof(type))
98
99@@ -3202,7 +3250,6 @@ MapSearch(QCBORDecodeContext *pMe,
100 if(uReturn != QCBOR_SUCCESS) {
101 goto Done;
102 }
103-
104 } while (uNextNestLevel >= uMapNestLevel);
105
106 uReturn = QCBOR_SUCCESS;
107@@ -3308,7 +3355,7 @@ static QCBORError
108 CheckTypeList(int uDataType, const uint8_t puTypeList[QCBOR_TAGSPEC_NUM_TYPES])
109 {
110 for(size_t i = 0; i < QCBOR_TAGSPEC_NUM_TYPES; i++) {
111- if(uDataType == puTypeList[i]) {
112+ if(uDataType == puTypeList[i]) { /* -Wmaybe-uninitialized falsly warns here */
113 return QCBOR_SUCCESS;
114 }
115 }
116@@ -3344,10 +3391,11 @@ CheckTypeList(int uDataType, const uint8_t puTypeList[QCBOR_TAGSPEC_NUM_TYPES])
117 static QCBORError
118 CheckTagRequirement(const TagSpecification TagSpec, const QCBORItem *pItem)
119 {
120- const int nItemType = pItem->uDataType;
121+ const int nItemType = pItem->uDataType; /* -Wmaybe-uninitialized falsly warns here */
122 const int nTagReq = TagSpec.uTagRequirement & ~QCBOR_TAG_REQUIREMENT_ALLOW_ADDITIONAL_TAGS;
123
124 #ifndef QCBOR_DISABLE_TAGS
125+ /* -Wmaybe-uninitialized falsly warns here */
126 if(!(TagSpec.uTagRequirement & QCBOR_TAG_REQUIREMENT_ALLOW_ADDITIONAL_TAGS) &&
127 pItem->uTags[0] != CBOR_TAG_INVALID16) {
128 /* There are tags that QCBOR couldn't process on this item and
129@@ -5189,7 +5237,7 @@ void QCBORDecode_GetUInt64ConvertInternalInMapSZ(QCBORDecodeContext *pMe,
130 static QCBORError
131 UInt64ConvertAll(const QCBORItem *pItem, uint32_t uConvertTypes, uint64_t *puValue)
132 {
133- switch(pItem->uDataType) {
134+ switch(pItem->uDataType) { /* -Wmaybe-uninitialized falsly warns here */
135
136 case QCBOR_TYPE_POSBIGNUM:
137 if(uConvertTypes & QCBOR_CONVERT_TYPE_BIG_NUM) {
138--
1392.25.1
140