* [PATCH 06/16] dyndbg: fix a BUG_ON in ddebug_describe_flags
@ 2019-11-27 17:50 Jim Cromie
2019-12-02 17:37 ` kbuild test robot
0 siblings, 1 reply; 3+ messages in thread
From: Jim Cromie @ 2019-11-27 17:50 UTC (permalink / raw)
To: jbaron, linux-kernel; +Cc: linux, greg, Jim Cromie
ddebug_describe_flags currently fills a caller provided string buffer,
after testing its size (also passed) in a BUG_ON.
Fix this with a struct containing a known-big-enough string buffer,
and passing it instead.
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
lib/dynamic_debug.c | 23 +++++++++++------------
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index b5fb0aa0fbc3..0e4783e11755 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -62,6 +62,8 @@ struct ddebug_iter {
unsigned int idx;
};
+struct flagsbuf { char buf[12]; }; /* big enough to hold all the flags */
+
static DEFINE_MUTEX(ddebug_lock);
static LIST_HEAD(ddebug_tables);
static int verbose;
@@ -88,21 +90,19 @@ static struct { unsigned flag:8; char opt_char; } opt_array[] = {
};
/* format a string into buf[] which describes the _ddebug's flags */
-static char *ddebug_describe_flags(struct _ddebug *dp, char *buf,
- size_t maxlen)
+static char *ddebug_describe_flags(unsigned int flags, struct flagsbuf *fb)
{
- char *p = buf;
+ char *p = fb->buf;
int i;
- BUG_ON(maxlen < 6);
for (i = 0; i < ARRAY_SIZE(opt_array); ++i)
- if (dp->flags & opt_array[i].flag)
+ if (flags & opt_array[i].flag)
*p++ = opt_array[i].opt_char;
- if (p == buf)
+ if (p == fb->buf)
*p++ = '_';
*p = '\0';
- return buf;
+ return fb->buf;
}
#define vnpr_info(lvl, fmt, ...) \
@@ -148,7 +148,7 @@ static int ddebug_change(const struct ddebug_query *query,
struct ddebug_table *dt;
unsigned int newflags;
unsigned int nfound = 0;
- char flagbuf[10];
+ struct flagsbuf flags;
/* search for matching ddebugs */
mutex_lock(&ddebug_lock);
@@ -205,8 +205,7 @@ static int ddebug_change(const struct ddebug_query *query,
vpr_info("changed %s:%d [%s]%s =%s\n",
trim_prefix(dp->filename), dp->lineno,
dt->mod_name, dp->function,
- ddebug_describe_flags(dp, flagbuf,
- sizeof(flagbuf)));
+ ddebug_describe_flags(dp->flags, &flags));
}
}
mutex_unlock(&ddebug_lock);
@@ -820,7 +819,7 @@ static int ddebug_proc_show(struct seq_file *m, void *p)
{
struct ddebug_iter *iter = m->private;
struct _ddebug *dp = p;
- char flagsbuf[10];
+ struct flagsbuf flags;
v9pr_info("called m=%p p=%p\n", m, p);
@@ -833,7 +832,7 @@ static int ddebug_proc_show(struct seq_file *m, void *p)
seq_printf(m, "%s:%u [%s]%s =%s \"",
trim_prefix(dp->filename), dp->lineno,
iter->table->mod_name, dp->function,
- ddebug_describe_flags(dp, flagsbuf, sizeof(flagsbuf)));
+ ddebug_describe_flags(dp->flags, &flags));
seq_escape(m, dp->format, "\t\r\n\"");
seq_puts(m, "\"\n");
--
2.23.0
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 06/16] dyndbg: fix a BUG_ON in ddebug_describe_flags
2019-11-27 17:50 [PATCH 06/16] dyndbg: fix a BUG_ON in ddebug_describe_flags Jim Cromie
@ 2019-12-02 17:37 ` kbuild test robot
0 siblings, 0 replies; 3+ messages in thread
From: kbuild test robot @ 2019-12-02 17:37 UTC (permalink / raw)
To: Jim Cromie; +Cc: kbuild-all, jbaron, linux-kernel, linux, greg, Jim Cromie
[-- Attachment #1: Type: text/plain, Size: 4951 bytes --]
Hi Jim,
I love your patch! Yet something to improve:
[auto build test ERROR on jeyu/modules-next]
[also build test ERROR on v5.4 next-20191202]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Jim-Cromie/dynamic-debug-cleanups-2-new-features/20191128-023731
base: https://git.kernel.org/pub/scm/linux/kernel/git/jeyu/linux.git modules-next
config: x86_64-rhel-7.6 (attached as .config)
compiler: gcc-7 (Debian 7.5.0-1) 7.5.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
Note: the linux-review/Jim-Cromie/dynamic-debug-cleanups-2-new-features/20191128-023731 HEAD 89f783b27d95e352559b645885086a41e514b5eb builds fine.
It only hurts bisectibility.
All errors (new ones prefixed by >>):
lib/dynamic_debug.c: In function 'ddebug_change':
>> lib/dynamic_debug.c:151:18: error: 'flags' redeclared as different kind of symbol
struct flagsbuf flags;
^~~~~
lib/dynamic_debug.c:145:17: note: previous definition of 'flags' was here
unsigned int flags, unsigned int mask)
^~~~~
>> lib/dynamic_debug.c:194:34: error: invalid operands to binary | (have 'unsigned int' and 'struct flagsbuf')
newflags = (dp->flags & mask) | flags;
~~~~~~~~~~~~~~~~~~ ^
>> lib/dynamic_debug.c:199:17: error: invalid operands to binary & (have 'struct flagsbuf' and 'int')
if (!(flags & _DPRINTK_FLAGS_PRINT))
^
lib/dynamic_debug.c:201:21: error: invalid operands to binary & (have 'struct flagsbuf' and 'int')
} else if (flags & _DPRINTK_FLAGS_PRINT)
^
vim +/flags +151 lib/dynamic_debug.c
137
138 /*
139 * Search the tables for _ddebug's which match the given `query' and
140 * apply the `flags' and `mask' to them. Returns number of matching
141 * callsites, normally the same as number of changes. If verbose,
142 * logs the changes. Takes ddebug_lock.
143 */
144 static int ddebug_change(const struct ddebug_query *query,
145 unsigned int flags, unsigned int mask)
146 {
147 int i;
148 struct ddebug_table *dt;
149 unsigned int newflags;
150 unsigned int nfound = 0;
> 151 struct flagsbuf flags;
152
153 /* search for matching ddebugs */
154 mutex_lock(&ddebug_lock);
155 list_for_each_entry(dt, &ddebug_tables, link) {
156
157 /* match against the module name */
158 if (query->module &&
159 !match_wildcard(query->module, dt->mod_name))
160 continue;
161
162 for (i = 0; i < dt->num_ddebugs; i++) {
163 struct _ddebug *dp = &dt->ddebugs[i];
164
165 /* match against the source filename */
166 if (query->filename &&
167 !match_wildcard(query->filename, dp->filename) &&
168 !match_wildcard(query->filename,
169 kbasename(dp->filename)) &&
170 !match_wildcard(query->filename,
171 trim_prefix(dp->filename)))
172 continue;
173
174 /* match against the function */
175 if (query->function &&
176 !match_wildcard(query->function, dp->function))
177 continue;
178
179 /* match against the format */
180 if (query->format &&
181 !strstr(dp->format, query->format))
182 continue;
183
184 /* match against the line number range */
185 if (query->first_lineno &&
186 dp->lineno < query->first_lineno)
187 continue;
188 if (query->last_lineno &&
189 dp->lineno > query->last_lineno)
190 continue;
191
192 nfound++;
193
> 194 newflags = (dp->flags & mask) | flags;
195 if (newflags == dp->flags)
196 continue;
197 #ifdef CONFIG_JUMP_LABEL
198 if (dp->flags & _DPRINTK_FLAGS_PRINT) {
> 199 if (!(flags & _DPRINTK_FLAGS_PRINT))
200 static_branch_disable(&dp->key.dd_key_true);
201 } else if (flags & _DPRINTK_FLAGS_PRINT)
202 static_branch_enable(&dp->key.dd_key_true);
203 #endif
204 dp->flags = newflags;
205 vpr_info("changed %s:%d [%s]%s =%s\n",
206 trim_prefix(dp->filename), dp->lineno,
207 dt->mod_name, dp->function,
208 ddebug_describe_flags(dp->flags, &flags));
209 }
210 }
211 mutex_unlock(&ddebug_lock);
212
213 if (!nfound && verbose)
214 pr_info("no matches for query\n");
215
216 return nfound;
217 }
218
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 48308 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH 06/16] dyndbg: fix a BUG_ON in ddebug_describe_flags
2020-06-05 16:26 [PATCH 00/16] dynamic_debug: cleanups, 2 features Jim Cromie
@ 2020-06-05 16:26 ` Jim Cromie
0 siblings, 0 replies; 3+ messages in thread
From: Jim Cromie @ 2020-06-05 16:26 UTC (permalink / raw)
To: jbaron, linux-kernel, akpm, gregkh; +Cc: linux, Jim Cromie
ddebug_describe_flags currently fills a caller provided string buffer,
after testing its size (also passed) in a BUG_ON. Fix this by
replacing them with a known-big-enough string buffer wrapped in a
struct, and passing that instead.
Also simplify the flags parameter, and instead de-ref the flags struct
in the caller; this makes the function reusable (soon) where flags are
unpacked.
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
lib/dynamic_debug.c | 31 +++++++++++++++----------------
1 file changed, 15 insertions(+), 16 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 25cc1eb86d96..08b8c9c04a17 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -87,22 +87,22 @@ static struct { unsigned flag:8; char opt_char; } opt_array[] = {
{ _DPRINTK_FLAGS_NONE, '_' },
};
+struct flagsbuf { char buf[ARRAY_SIZE(opt_array)+1]; };
+
/* format a string into buf[] which describes the _ddebug's flags */
-static char *ddebug_describe_flags(struct _ddebug *dp, char *buf,
- size_t maxlen)
+static char *ddebug_describe_flags(unsigned int flags, struct flagsbuf *fb)
{
- char *p = buf;
+ char *p = fb->buf;
int i;
- BUG_ON(maxlen < 6);
for (i = 0; i < ARRAY_SIZE(opt_array); ++i)
- if (dp->flags & opt_array[i].flag)
+ if (flags & opt_array[i].flag)
*p++ = opt_array[i].opt_char;
- if (p == buf)
+ if (p == fb->buf)
*p++ = '_';
*p = '\0';
- return buf;
+ return fb->buf;
}
#define vnpr_info(lvl, fmt, ...) \
@@ -141,13 +141,13 @@ static void vpr_info_dq(const struct ddebug_query *query, const char *msg)
* logs the changes. Takes ddebug_lock.
*/
static int ddebug_change(const struct ddebug_query *query,
- unsigned int flags, unsigned int mask)
+ unsigned int pflags, unsigned int mask)
{
int i;
struct ddebug_table *dt;
unsigned int newflags;
unsigned int nfound = 0;
- char flagbuf[10];
+ struct flagsbuf flags;
/* search for matching ddebugs */
mutex_lock(&ddebug_lock);
@@ -190,22 +190,21 @@ static int ddebug_change(const struct ddebug_query *query,
nfound++;
- newflags = (dp->flags & mask) | flags;
+ newflags = (dp->flags & mask) | pflags;
if (newflags == dp->flags)
continue;
#ifdef CONFIG_JUMP_LABEL
if (dp->flags & _DPRINTK_FLAGS_PRINT) {
- if (!(flags & _DPRINTK_FLAGS_PRINT))
+ if (!(pflags & _DPRINTK_FLAGS_PRINT))
static_branch_disable(&dp->key.dd_key_true);
- } else if (flags & _DPRINTK_FLAGS_PRINT)
+ } else if (pflags & _DPRINTK_FLAGS_PRINT)
static_branch_enable(&dp->key.dd_key_true);
#endif
dp->flags = newflags;
v2pr_info("changed %s:%d [%s]%s =%s\n",
trim_prefix(dp->filename), dp->lineno,
dt->mod_name, dp->function,
- ddebug_describe_flags(dp, flagbuf,
- sizeof(flagbuf)));
+ ddebug_describe_flags(dp->flags, &flags));
}
}
mutex_unlock(&ddebug_lock);
@@ -814,7 +813,7 @@ static int ddebug_proc_show(struct seq_file *m, void *p)
{
struct ddebug_iter *iter = m->private;
struct _ddebug *dp = p;
- char flagsbuf[10];
+ struct flagsbuf flags;
if (p == SEQ_START_TOKEN) {
seq_puts(m,
@@ -825,7 +824,7 @@ static int ddebug_proc_show(struct seq_file *m, void *p)
seq_printf(m, "%s:%u [%s]%s =%s \"",
trim_prefix(dp->filename), dp->lineno,
iter->table->mod_name, dp->function,
- ddebug_describe_flags(dp, flagsbuf, sizeof(flagsbuf)));
+ ddebug_describe_flags(dp->flags, &flags));
seq_escape(m, dp->format, "\t\r\n\"");
seq_puts(m, "\"\n");
--
2.26.2
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-06-05 16:27 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-27 17:50 [PATCH 06/16] dyndbg: fix a BUG_ON in ddebug_describe_flags Jim Cromie
2019-12-02 17:37 ` kbuild test robot
2020-06-05 16:26 [PATCH 00/16] dynamic_debug: cleanups, 2 features Jim Cromie
2020-06-05 16:26 ` [PATCH 06/16] dyndbg: fix a BUG_ON in ddebug_describe_flags Jim Cromie
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®