Skip to content
Snippets Groups Projects
  • Jakub Jelinek's avatar
    4963eb76
    libcpp: Fix up UB in finish_embed · 4963eb76
    Jakub Jelinek authored
    Jonathan reported on IRC that certain unnamed proprietary static analyzer
    is unhappy about the new finish_embed function and it is actually right.
    On a testcase like:
     #embed __FILE__ limit (0) if_empty (0)
    params->if_empty.count is 1, limit is 0, so count is 0 (we need just
    a single token and one fits into pfile->directive_result).  Because
    count is 0, we don't allocate toks, so it stays NULL, and then in
    1301      if (prefix->count)
    1302        {
    1303          *tok = *prefix->base_run.base;
    1304          tok = toks;
    1305          tokenrun *cur_run = &prefix->base_run;
    1306          while (cur_run)
    1307            {
    1308              size_t cnt = (cur_run->next ? cur_run->limit
    1309                            : prefix->cur_token) - cur_run->base;
    1310              cpp_token *t = cur_run->base;
    1311              if (cur_run == &prefix->base_run)
    1312                {
    1313                  t++;
    1314                  cnt--;
    1315                }
    1316              memcpy (tok, t, cnt * sizeof (cpp_token));
    1317              tok += cnt;
    1318              cur_run = cur_run->next;
    1319            }
    1320        }
    the *tok = *prefix->base_run.base; assignment will copy the only
    token.  cur_run is still non-NULL, cnt will be initially 1 and
    then decremented to 0, but we invoke UB because we do
    memcpy (NULL, cur_run->base + 1, 0 * sizeof (cpp_token));
    and then the loop stops because cur_run->next must be NULL.
    
    As we don't really copy anything, toks can be anything non-NULL,
    so the following patch fixes that by initializing toks also to
    &pfile->directive_result (just something known to be non-NULL).
    This should be harmless even for the
     #embed __FILE__ limit (1)
    case (no non-empty prefix/suffix) where toks isn't allocated
    either, but in that case prefix->count will be 0 and in the
    1321      for (size_t i = 0; i < limit; ++i)
    1322        {
    1323          tok->src_loc = params->loc;
    1324          tok->type = CPP_NUMBER;
    1325          tok->flags = NO_EXPAND;
    1326          if (i == 0)
    1327            tok->flags |= PREV_WHITE;
    1328          tok->val.str.text = s;
    1329          tok->val.str.len = sprintf ((char *) s, "%d", buffer[i]);
    1330          s += tok->val.str.len + 1;
    1331          if (tok == &pfile->directive_result)
    1332            tok = toks;
    1333          else
    1334            tok++;
    1335          if (i < limit - 1)
    1336            {
    1337              tok->src_loc = params->loc;
    1338              tok->type = CPP_COMMA;
    1339              tok->flags = NO_EXPAND;
    1340              tok++;
    1341            }
    1342        }
    loop limit will be 1, so tok is initially &pfile->directive_result,
    that is stilled in, then tok = toks; (previously setting tok to NULL,
    now to &pfile->directive_result again) and because 0 < 1 - 1 is
    false, nothing further will happen and the loop will finish (and as
    params->suffix.count will be 0, nothing further will use tok).
    
    2024-09-13  Jakub Jelinek  <jakub@redhat.com>
    
    	* files.cc (finish_embed): Initialize toks to tok rather
    	than NULL.
    4963eb76
    History
    libcpp: Fix up UB in finish_embed
    Jakub Jelinek authored
    Jonathan reported on IRC that certain unnamed proprietary static analyzer
    is unhappy about the new finish_embed function and it is actually right.
    On a testcase like:
     #embed __FILE__ limit (0) if_empty (0)
    params->if_empty.count is 1, limit is 0, so count is 0 (we need just
    a single token and one fits into pfile->directive_result).  Because
    count is 0, we don't allocate toks, so it stays NULL, and then in
    1301      if (prefix->count)
    1302        {
    1303          *tok = *prefix->base_run.base;
    1304          tok = toks;
    1305          tokenrun *cur_run = &prefix->base_run;
    1306          while (cur_run)
    1307            {
    1308              size_t cnt = (cur_run->next ? cur_run->limit
    1309                            : prefix->cur_token) - cur_run->base;
    1310              cpp_token *t = cur_run->base;
    1311              if (cur_run == &prefix->base_run)
    1312                {
    1313                  t++;
    1314                  cnt--;
    1315                }
    1316              memcpy (tok, t, cnt * sizeof (cpp_token));
    1317              tok += cnt;
    1318              cur_run = cur_run->next;
    1319            }
    1320        }
    the *tok = *prefix->base_run.base; assignment will copy the only
    token.  cur_run is still non-NULL, cnt will be initially 1 and
    then decremented to 0, but we invoke UB because we do
    memcpy (NULL, cur_run->base + 1, 0 * sizeof (cpp_token));
    and then the loop stops because cur_run->next must be NULL.
    
    As we don't really copy anything, toks can be anything non-NULL,
    so the following patch fixes that by initializing toks also to
    &pfile->directive_result (just something known to be non-NULL).
    This should be harmless even for the
     #embed __FILE__ limit (1)
    case (no non-empty prefix/suffix) where toks isn't allocated
    either, but in that case prefix->count will be 0 and in the
    1321      for (size_t i = 0; i < limit; ++i)
    1322        {
    1323          tok->src_loc = params->loc;
    1324          tok->type = CPP_NUMBER;
    1325          tok->flags = NO_EXPAND;
    1326          if (i == 0)
    1327            tok->flags |= PREV_WHITE;
    1328          tok->val.str.text = s;
    1329          tok->val.str.len = sprintf ((char *) s, "%d", buffer[i]);
    1330          s += tok->val.str.len + 1;
    1331          if (tok == &pfile->directive_result)
    1332            tok = toks;
    1333          else
    1334            tok++;
    1335          if (i < limit - 1)
    1336            {
    1337              tok->src_loc = params->loc;
    1338              tok->type = CPP_COMMA;
    1339              tok->flags = NO_EXPAND;
    1340              tok++;
    1341            }
    1342        }
    loop limit will be 1, so tok is initially &pfile->directive_result,
    that is stilled in, then tok = toks; (previously setting tok to NULL,
    now to &pfile->directive_result again) and because 0 < 1 - 1 is
    false, nothing further will happen and the loop will finish (and as
    params->suffix.count will be 0, nothing further will use tok).
    
    2024-09-13  Jakub Jelinek  <jakub@redhat.com>
    
    	* files.cc (finish_embed): Initialize toks to tok rather
    	than NULL.