Problem report #13

Problem report #13

[Dan Nicolaescu]

CID: 13
Checker: FORWARD_NULL (help)
File: emacs/src/coding.c
Function: encode_coding_string
Description: Variable "(coding)->cmp_data" tracked as NULL was passed to a function that dereferences it.

Event var_compare_op: Added "(coding)->cmp_data" due to comparison "(coding)->cmp_data == 0"
Also see events: [var_deref_model]
At conditional (1): "(coding)->cmp_data == 0" taking true path

6435 	  if (coding->type != coding_type_ccl
6436 	      && (! coding->cmp_data || coding->cmp_data->used == 0))
6437 	    {

At conditional (2): "(to_byte - from) > shrink_conversion_region_threshhold" taking true path
At conditional (3): "1" taking true path
At conditional (4): "0" taking false path

6438 	      SHRINK_CONVERSION_REGION (&from, &to_byte, coding, SDATA (str),
6439 					1);

At conditional (5): "from == to_byte" taking false path

6440 	      if (from == to_byte)
6441 		{
6442 		  coding_free_composition_data (coding);
6443 		  goto no_need_of_encoding;
6444 		}

At conditional (6): "((0), (str & -8))->size_byte < 0" taking true path

6445 	      shrinked_bytes = from + (SBYTES (str) - to_byte);
6446 	    }
6447 	
6448 	  len = encoding_buffer_size (coding, to_byte - from);

At conditional (7): "len < 16384" taking true path
At conditional (8): "0" taking false path

6449 	  allocate_conversion_buffer (buf, len);
6450 	
6451 	  consumed = consumed_char = produced = produced_char = 0;

At conditional (9): "1" taking true path

6452 	  while (1)
6453 	    {

Event var_deref_model: Variable "(coding)->cmp_data" tracked as NULL was passed to a function that dereferences it. [model]
Also see events: [var_compare_op]

6454 	      result = encode_coding (coding, SDATA (str) + from + consumed,
6455 				      buf.data + produced, to_byte - from - consumed,
6456 				      buf.size - produced);

Re: Problem report #13 FALSE

[Kenichi Handa]

Dan Nicolaescu <dann@ics.uci.edu> writes:

> CID: 13
> Checker: FORWARD_NULL (help)
> File: emacs/src/coding.c
> Function: encode_coding_string
> Description: Variable "(coding)->cmp_data" tracked as NULL was passed to a function that dereferences it.

I see no bug here.   A member of (coding)->cmp_data is
accessed only after checking (coding)->cmp_data is not NULL.

> Event var_compare_op: Added "(coding)->cmp_data" due to comparison "(coding)->cmp_data == 0"
> Also see events: [var_deref_model]
> At conditional (1): "(coding)->cmp_data == 0" taking true path

> 6435 	  if (coding->type != coding_type_ccl
> 6436 	      && (! coding->cmp_data || coding->cmp_data->used == 0))
> 6437 	    {

Does the checker require it to be writen as below?

> 6436 	      && (coding->cmp_data ? coding->cmp_data->used == 0 : 1))

---
Kenichi Handa
handa@m17n.org

Re: Problem report #13 FALSE

[Dan Nicolaescu]

Kenichi Handa <handa@m17n.org> writes:

  > > Event var_compare_op: Added "(coding)->cmp_data" due to comparison "(coding)->cmp_data == 0"
  > > Also see events: [var_deref_model]
  > > At conditional (1): "(coding)->cmp_data == 0" taking true path
  >
  > > 6435 	  if (coding->type != coding_type_ccl
  > > 6436 	      && (! coding->cmp_data || coding->cmp_data->used == 0))
  > > 6437 	    {
  >
  > Does the checker require it to be writen as below?
  >
  > > 6436 	      && (coding->cmp_data ? coding->cmp_data->used == 0 : 1))

I don't know... 

Re: Problem report #13 FALSE

[Andreas Schwab]

Kenichi Handa <handa@m17n.org> writes:

> Dan Nicolaescu <dann@ics.uci.edu> writes:
>
>> CID: 13
>> Checker: FORWARD_NULL (help)
>> File: emacs/src/coding.c
>> Function: encode_coding_string
>> Description: Variable "(coding)->cmp_data" tracked as NULL was passed to a function that dereferences it.
>
> I see no bug here.   A member of (coding)->cmp_data is
> accessed only after checking (coding)->cmp_data is not NULL.
>
>> Event var_compare_op: Added "(coding)->cmp_data" due to comparison "(coding)->cmp_data == 0"
>> Also see events: [var_deref_model]
>> At conditional (1): "(coding)->cmp_data == 0" taking true path
>
>> 6435 	  if (coding->type != coding_type_ccl
>> 6436 	      && (! coding->cmp_data || coding->cmp_data->used == 0))
>> 6437 	    {
>
> Does the checker require it to be writen as below?
>
>> 6436 	      && (coding->cmp_data ? coding->cmp_data->used == 0 : 1))

That would be stupid, since it would be a major obfucation.  But that's
not the point of the report, the derefence is supposed to happen in
encode_coding.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: Problem report #13 FALSE

[Jason Rumney]

Kenichi Handa <handa@m17n.org> writes:

>> Function: encode_coding_string
>> Description: Variable "(coding)->cmp_data" tracked as NULL was passed to a function that dereferences it.
>
> I see no bug here.   A member of (coding)->cmp_data is
> accessed only after checking (coding)->cmp_data is not NULL.
>
>> Event var_compare_op: Added "(coding)->cmp_data" due to comparison "(coding)->cmp_data == 0"
>> Also see events: [var_deref_model]
>> At conditional (1): "(coding)->cmp_data == 0" taking true path
>
>> 6435 	  if (coding->type != coding_type_ccl
>> 6436 	      && (! coding->cmp_data || coding->cmp_data->used == 0))
>> 6437 	    {
>
> Does the checker require it to be writen as below?
>
>> 6436 	      && (coding->cmp_data ? coding->cmp_data->used == 0 : 1))

I don't think so, as that is not the line it is complaining
about. Rather the line:

6454          result = encode_coding (coding, SDATA (str) + from + consumed,
6455                                  buf.data + produced, to_byte - from - 
consumed,
6456                                  buf.size - produced);

So the question is,
1. Do we reach that line when coding->cmp_data is NULL?
2. Does coding->cmp_data really get dereferenced inside encode_coding
   in the case where it is NULL?

Judging by your previous responses to coding related problem reports,
there is probably some other condition that always prevents both of
these, but I don't understand enough about the coding structure to
tell myself.

Re: Problem report #13 FALSE

[Kenichi Handa]

In article <jer730zz5x.fsf@sykes.suse.de>, Andreas Schwab <schwab@suse.de> writes:

> That would be stupid, since it would be a major obfucation.  But that's
> not the point of the report, the derefence is supposed to happen in
> encode_coding.

But, it seems that the derefence happens only where
coding->cmp_data is not NULL or coding->composing is not
COMPOSITION_DISABLED.  And if coding->composing is not
COMPOSITION_DISABLED, we surely allocate coding->cmp_data.

---
Kenichi Handa
handa@m17n.org