Re: Patch 8.1.0565

2018-12-06 Fir de Conversatie Bram Moolenaar


Dominique wrote:

> Bram Moolenaar wrote:
> 
> > I wrote:
> >
> > > Patch 8.1.0565
> > > Problem:Asan complains about reading before allocated block.
> > > Solution:   Workaround: Avoid offset from becoming negative.
> > > Files:src/gui.c
> > >
> > >
> > > *** ../vim-8.1.0564/src/gui.c 2018-11-16 16:21:01.633310065 +0100
> > > --- src/gui.c 2018-12-05 19:44:07.455956642 +0100
> > > ***
> > > *** 2753,2759 
> > >   }
> > >   else if (enc_utf8)
> > >   {
> > > ! if (ScreenLines[off + col1] == 0)
> > >   --col1;
> > >   # ifdef FEAT_GUI_GTK
> > >   if (col2 + 1 < Columns && ScreenLines[off + col2 + 1] == 0)
> > > --- 2753,2760 
> > >   }
> > >   else if (enc_utf8)
> > >   {
> > > ! // FIXME: how can the first character ever be zero?
> > > ! if (col1 > 0 && ScreenLines[off + col1] == 0)
> > >   --col1;
> > >   # ifdef FEAT_GUI_GTK
> > >   if (col2 + 1 < Columns && ScreenLines[off + col2 + 1] == 0)
> >
> > This indeed fixed the ASAN report.  However, I don't see how the
> > character in the first column can be zero.  That should not happen.
> >
> > The ASAN reporte started with patch 8.1.0562, which changes the parsing
> > of 'diffmode'.  I don't see how that can trigger this problem.  It might
> > have been caused by a library change.
> >
> > I have not been able to reproduce the problem locally.  If someone can,
> > please figure out what the root cause is.  E.g. would be useful to know
> > what is being redrawn and what is in the other lines.  I suspect it's
> > redrawing the whole display, so perhaps it's the last line?
> 
> For the record, all tests pass for me locally with
> asan too, despite trying to undo the workaround
> from patch 8.1.0565.

I suspect it's something in the setup on Travis that triggers it.
Hard to tell what it is.

I thought valgrind had a flag to check for access before allocated
memory (it normally checks for access after allocated memory), but can't
find it...  Perhaps it does check both always?

-- 
I used to wonder about the meaning of life.  But I looked it
up in the dictionary under "L" and there it was - the meaning
of life.  It was less than I expected.  - Dogbert

 /// Bram Moolenaar -- b...@moolenaar.net -- http://www.Moolenaar.net   \\\
///sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\  an exciting new programming language -- http://www.Zimbu.org///
 \\\help me help AIDS victims -- http://ICCF-Holland.org///

-- 
-- 
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php

--- 
You received this message because you are subscribed to the Google Groups 
"vim_dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to vim_dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: Patch 8.1.0565

2018-12-05 Fir de Conversatie Dominique Pellé
Bram Moolenaar wrote:

> I wrote:
>
> > Patch 8.1.0565
> > Problem:Asan complains about reading before allocated block.
> > Solution:   Workaround: Avoid offset from becoming negative.
> > Files:src/gui.c
> >
> >
> > *** ../vim-8.1.0564/src/gui.c 2018-11-16 16:21:01.633310065 +0100
> > --- src/gui.c 2018-12-05 19:44:07.455956642 +0100
> > ***
> > *** 2753,2759 
> >   }
> >   else if (enc_utf8)
> >   {
> > ! if (ScreenLines[off + col1] == 0)
> >   --col1;
> >   # ifdef FEAT_GUI_GTK
> >   if (col2 + 1 < Columns && ScreenLines[off + col2 + 1] == 0)
> > --- 2753,2760 
> >   }
> >   else if (enc_utf8)
> >   {
> > ! // FIXME: how can the first character ever be zero?
> > ! if (col1 > 0 && ScreenLines[off + col1] == 0)
> >   --col1;
> >   # ifdef FEAT_GUI_GTK
> >   if (col2 + 1 < Columns && ScreenLines[off + col2 + 1] == 0)
>
> This indeed fixed the ASAN report.  However, I don't see how the
> character in the first column can be zero.  That should not happen.
>
> The ASAN reporte started with patch 8.1.0562, which changes the parsing
> of 'diffmode'.  I don't see how that can trigger this problem.  It might
> have been caused by a library change.
>
> I have not been able to reproduce the problem locally.  If someone can,
> please figure out what the root cause is.  E.g. would be useful to know
> what is being redrawn and what is in the other lines.  I suspect it's
> redrawing the whole display, so perhaps it's the last line?

For the record, all tests pass for me locally with
asan too, despite trying to undo the workaround
from patch 8.1.0565.

Dominique

-- 
-- 
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php

--- 
You received this message because you are subscribed to the Google Groups 
"vim_dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to vim_dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: Patch 8.1.0565

2018-12-05 Fir de Conversatie Bram Moolenaar


Tony wrote:

> On Wed, Dec 5, 2018 at 7:46 PM Bram Moolenaar  wrote:
> >
> >
> > Patch 8.1.0565
> > Problem:Asan complains about reading before allocated block.
> > Solution:   Workaround: Avoid offset from becoming negative.
> > Files:  src/gui.c
> [...]
> > !   // FIXME: how can the first character ever be zero?
> > !   if (col1 > 0 && ScreenLines[off + col1] == 0)
> > --col1;
> [...]
> 
> Experiment shows that after yanking a block from an empty buffer (i.e.
> the ruler says "0,0-1") with 'virtualedit' defaulted to empty, the
> register yanked to is empty. Doesn't that mean that the
> string-terminating null byte is found at character zero of the block?
> 
> Of course, one would normally not yank from an empty buffer — except,
> maybe, to test that the program logic is working properly, even in the
> most absurd circumstances.

This is using ScreenLines[], which should contain spaces where the
display is blank.  Has nothing to do with register content or what text
is stored in the buffer.

-- 
Kiss me twice.  I'm schizophrenic.

 /// Bram Moolenaar -- b...@moolenaar.net -- http://www.Moolenaar.net   \\\
///sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\  an exciting new programming language -- http://www.Zimbu.org///
 \\\help me help AIDS victims -- http://ICCF-Holland.org///

-- 
-- 
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php

--- 
You received this message because you are subscribed to the Google Groups 
"vim_dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to vim_dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: Patch 8.1.0565

2018-12-05 Fir de Conversatie Bram Moolenaar


I wrote:

> Patch 8.1.0565
> Problem:Asan complains about reading before allocated block.
> Solution:   Workaround: Avoid offset from becoming negative.
> Files:src/gui.c
> 
> 
> *** ../vim-8.1.0564/src/gui.c 2018-11-16 16:21:01.633310065 +0100
> --- src/gui.c 2018-12-05 19:44:07.455956642 +0100
> ***
> *** 2753,2759 
>   }
>   else if (enc_utf8)
>   {
> ! if (ScreenLines[off + col1] == 0)
>   --col1;
>   # ifdef FEAT_GUI_GTK
>   if (col2 + 1 < Columns && ScreenLines[off + col2 + 1] == 0)
> --- 2753,2760 
>   }
>   else if (enc_utf8)
>   {
> ! // FIXME: how can the first character ever be zero?
> ! if (col1 > 0 && ScreenLines[off + col1] == 0)
>   --col1;
>   # ifdef FEAT_GUI_GTK
>   if (col2 + 1 < Columns && ScreenLines[off + col2 + 1] == 0)

This indeed fixed the ASAN report.  However, I don't see how the
character in the first column can be zero.  That should not happen.

The ASAN reporte started with patch 8.1.0562, which changes the parsing
of 'diffmode'.  I don't see how that can trigger this problem.  It might
have been caused by a library change.

I have not been able to reproduce the problem locally.  If someone can,
please figure out what the root cause is.  E.g. would be useful to know
what is being redrawn and what is in the other lines.  I suspect it's
redrawing the whole display, so perhaps it's the last line?

-- 
"I've been teaching myself to play the piano for about 5 years and now write
most of my songs on it, mainly because I can never find any paper."
Jeff Lynne, ELO's greatest hits

 /// Bram Moolenaar -- b...@moolenaar.net -- http://www.Moolenaar.net   \\\
///sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\  an exciting new programming language -- http://www.Zimbu.org///
 \\\help me help AIDS victims -- http://ICCF-Holland.org///

-- 
-- 
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php

--- 
You received this message because you are subscribed to the Google Groups 
"vim_dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to vim_dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: Patch 8.1.0565

2018-12-05 Fir de Conversatie Tony Mechelynck
On Wed, Dec 5, 2018 at 7:46 PM Bram Moolenaar  wrote:
>
>
> Patch 8.1.0565
> Problem:Asan complains about reading before allocated block.
> Solution:   Workaround: Avoid offset from becoming negative.
> Files:  src/gui.c
[...]
> !   // FIXME: how can the first character ever be zero?
> !   if (col1 > 0 && ScreenLines[off + col1] == 0)
> --col1;
[...]

Experiment shows that after yanking a block from an empty buffer (i.e.
the ruler says "0,0-1") with 'virtualedit' defaulted to empty, the
register yanked to is empty. Doesn't that mean that the
string-terminating null byte is found at character zero of the block?

Of course, one would normally not yank from an empty buffer — except,
maybe, to test that the program logic is working properly, even in the
most absurd circumstances.

Best regards,
Tony.

-- 
-- 
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php

--- 
You received this message because you are subscribed to the Google Groups 
"vim_dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to vim_dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Patch 8.1.0565

2018-12-05 Fir de Conversatie Bram Moolenaar


Patch 8.1.0565
Problem:Asan complains about reading before allocated block.
Solution:   Workaround: Avoid offset from becoming negative.
Files:  src/gui.c


*** ../vim-8.1.0564/src/gui.c   2018-11-16 16:21:01.633310065 +0100
--- src/gui.c   2018-12-05 19:44:07.455956642 +0100
***
*** 2753,2759 
}
else if (enc_utf8)
{
!   if (ScreenLines[off + col1] == 0)
--col1;
  # ifdef FEAT_GUI_GTK
if (col2 + 1 < Columns && ScreenLines[off + col2 + 1] == 0)
--- 2753,2760 
}
else if (enc_utf8)
{
!   // FIXME: how can the first character ever be zero?
!   if (col1 > 0 && ScreenLines[off + col1] == 0)
--col1;
  # ifdef FEAT_GUI_GTK
if (col2 + 1 < Columns && ScreenLines[off + col2 + 1] == 0)
*** ../vim-8.1.0564/src/version.c   2018-12-05 18:43:24.489493117 +0100
--- src/version.c   2018-12-05 19:45:06.855458016 +0100
***
*** 794,795 
--- 794,797 
  {   /* Add new patch number below this line */
+ /**/
+ 565,
  /**/

-- 
SECOND SOLDIER: It could be carried by an African swallow!
FIRST SOLDIER:  Oh  yes! An African swallow maybe ... but not a European
swallow. that's my point.
 "Monty Python and the Holy Grail" PYTHON (MONTY) PICTURES LTD

 /// Bram Moolenaar -- b...@moolenaar.net -- http://www.Moolenaar.net   \\\
///sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\  an exciting new programming language -- http://www.Zimbu.org///
 \\\help me help AIDS victims -- http://ICCF-Holland.org///

-- 
-- 
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php

--- 
You received this message because you are subscribed to the Google Groups 
"vim_dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to vim_dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.