composition bug

composition bug

[Chong Yidong]

Something in your 2008-09-05 composition changes broke navigation in the
HELLO file.  To reproduce:

emacs -Q
C-h H
C-n     -> cursor jumps to point 410.

Could you fix this?  Thanks!

Re: composition bug

[Kenichi Handa]

In article <87hc8rn3ws.fsf@cyd.mit.edu>, Chong Yidong <cyd@stupidchicken.com> writes:

> Something in your 2008-09-05 composition changes broke navigation in the
> HELLO file.  To reproduce:

> emacs -Q
> C-h H
> C-n     -> cursor jumps to point 410.

> Could you fix this?  Thanks!

Romain Francoise <romain@orebokech.com> writes:

> This change introduces some very strange behavior in tty mode.
> To see what I'm talking about, start Emacs with emacs -nw, do C-h h
> to display the HELLO file and try to move around with C-n or C-f.
> Point will move randomly in the buffer...

I can't reprodce the above behaviours but I fixed a bug in
vertical_motion.  Could you please try again with the latest
code?

---
Kenichi Handa
handa@ni.aist.go.jp

Re: composition bug

[Romain Francoise]

Kenichi Handa <handa@m17n.org> writes:

> I can't reprodce the above behaviours but I fixed a bug in
> vertical_motion.  Could you please try again with the latest
> code?

With current CVS HEAD, not only does it still move point randomly
like before, but if I keep moving it eventually segfaults:

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fd8ce7f9770 (LWP 11285)]
0x000000000069419d in fill_gstring_body (gstring=12363924) at composite.c:875
875           LGLYPH_SET_FROM (g, i);
(gdb) bt
#0  0x000000000069419d in fill_gstring_body (gstring=12363924)
    at composite.c:875
#1  0x000000000069803b in Fcomposition_get_gstring (from=12688, to=12760,
    font_object=11951841, string=11951841) at composite.c:1477
#2  0x000000000069475e in autocmp_chars (cft_element=31373157, charpos=1586,
    bytepos=2367, limit=3099, win=0xb961c0, face=0x1d0b3b0, string=11951841)
    at composite.c:958
#3  0x000000000069549a in composition_reseat_it (cmp_it=0x7fffd6929ba0,
    charpos=1586, bytepos=2367, endpos=3099, w=0xb961c0, face=0x1d0b3b0,
    string=11951841) at composite.c:1094
#4  0x000000000043d654 in next_element_from_buffer (it=0x7fffd6929600)
    at xdisp.c:6458
#5  0x000000000043b365 in get_next_display_element (it=0x7fffd6929600)
    at xdisp.c:5654
#6  0x0000000000457516 in display_line (it=0x7fffd6929600) at xdisp.c:16539
#7  0x000000000044fb15 in try_window (window=12149188, pos=
      {charpos = 1, bytepos = 1}, check_margins=1) at xdisp.c:14003
#8  0x000000000044e5ca in redisplay_window (window=12149188, just_this_one_p=0)
    at xdisp.c:13626
#9  0x0000000000449f9c in redisplay_window_0 (window=12149188) at xdisp.c:12216
#10 0x00000000006127ed in internal_condition_case_1 (
    bfun=0x449f60 <redisplay_window_0>, arg=12149188, handlers=12307637,
    hfun=0x449f35 <redisplay_window_error>) at eval.c:1559
#11 0x0000000000449f16 in redisplay_windows (window=12149188) at xdisp.c:12195
#12 0x00000000004490c9 in redisplay_internal (preserve_echo_area=0)
    at xdisp.c:11771
#13 0x0000000000446edd in redisplay () at xdisp.c:10977
#14 0x0000000000575680 in read_char (commandflag=1, nmaps=5,
    maps=0x7fffd692b9e0, prev_event=11951841, used_mouse_menu=0x7fffd692bd24,
    end_time=0x0) at keyboard.c:2649
#15 0x000000000058219f in read_key_sequence (keybuf=0x7fffd692c090,
    bufsize=30, prompt=11951841, dont_downcase_last=0,
    can_return_switch_frame=1, fix_current_buffer=1) at keyboard.c:9346
#16 0x00000000005724b9 in command_loop_1 () at keyboard.c:1621
#17 0x0000000000612651 in internal_condition_case (
    bfun=0x57211f <command_loop_1>, handlers=12039057,
    hfun=0x571a87 <cmd_error>) at eval.c:1511
#18 0x0000000000571e3e in command_loop_2 () at keyboard.c:1338
#19 0x0000000000612003 in internal_catch (tag=12020353,
    func=0x571e24 <command_loop_2>, arg=11951841) at eval.c:1247
#20 0x0000000000571dfe in command_loop () at keyboard.c:1317
#21 0x00000000005715cd in recursive_edit_1 () at keyboard.c:942
#22 0x0000000000571770 in Frecursive_edit () at keyboard.c:1004
#23 0x000000000056fd6a in main (argc=2, argv=0x7fffd692c9b8) at emacs.c:1693

Re: composition bug

[Kenichi Handa]

In article <87r67u3k52.fsf@elegiac.orebokech.com>, Romain Francoise <romain@orebokech.com> writes:

> With current CVS HEAD, not only does it still move point randomly
> like before, but if I keep moving it eventually segfaults:

> Program received signal SIGSEGV, Segmentation fault.
> [Switching to Thread 0x7fd8ce7f9770 (LWP 11285)]
> 0x000000000069419d in fill_gstring_body (gstring=12363924) at composite.c:875
> 875           LGLYPH_SET_FROM (g, i);
> (gdb) bt
> #0  0x000000000069419d in fill_gstring_body (gstring=12363924)
>     at composite.c:875

I've just fixed a stupid bug in composite.c.  Please try
again.

---
Kenichi Handa
handa@ni.aist.go.jp

Re: composition bug

[Romain Francoise]

Kenichi Handa <handa@m17n.org> writes:

> I've just fixed a stupid bug in composite.c.  Please try
> again.

Thanks, it fixes the segfault but the original problem with cursor
movement is still present.  I'm surprised that you cannot reproduce
it.  What could be a factor in this?  My locale (en_US.UTF-8)?  The
fact that I'm building without libotf?

Re: composition bug

[Kenichi Handa]

In article <87iqt53mk7.fsf@elegiac.orebokech.com>, Romain Francoise <romain@orebokech.com> writes:

> Thanks, it fixes the segfault but the original problem with cursor
> movement is still present.  I'm surprised that you cannot reproduce
> it.  What could be a factor in this?  My locale (en_US.UTF-8)?  The
> fact that I'm building without libotf?

Ah, perhaps that is.  I'll try without libotf too.

---
Kenichi Handa
handa@ni.aist.go.jp

Re: composition bug

[Juanma Barranquero]

> Ah, perhaps that is.  I'll try without libotf too.

Bug #874 is also related to composition. Are you able to reproduce it?

 Juanma

Re: composition bug

[Kenichi Handa]

In article <E1KcxWB-0006Ur-2N@etlken.m17n.org>, Kenichi Handa <handa@m17n.org> writes:

> In article <87iqt53mk7.fsf@elegiac.orebokech.com>, Romain Francoise <romain@orebokech.com> writes:
> > Thanks, it fixes the segfault but the original problem with cursor
> > movement is still present.  I'm surprised that you cannot reproduce
> > it.  What could be a factor in this?  My locale (en_US.UTF-8)?  The
> > fact that I'm building without libotf?

Do you still see this bug with the latest code?

---
Kenichi Handa
handa@ni.aist.go.jp

Re: composition bug

[Romain Francoise]

Kenichi Handa <handa@m17n.org> writes:

> Do you still see this bug with the latest code?

Yes, both in X and the console.

Re: composition bug

[Romain Francoise]

I just noticed something about this bug: if I save all the text from
the HELLO buffer and insert it into another, it doesn't happen
anymore.  In the new buffer, the cursor moves normally.

(E.g. do C-h h C-x h M-w C-x b foo RET C-y.)

However, as soon as I save this new buffer to a file the bug appears
again and point moves to random positions in the buffer with C-n/C-f.
I tried various coding systems for saving the file, but it doesn't
seem to be relevant.

Does that help you diagnose the problem?

Thanks.

Re: composition bug

[Kenichi Handa]

In article <87zlmcxsq1.fsf@elegiac.orebokech.com>, Romain Francoise <romain@orebokech.com> writes:

> I just noticed something about this bug: if I save all the text from
> the HELLO buffer and insert it into another, it doesn't happen
> anymore.  In the new buffer, the cursor moves normally.

> (E.g. do C-h h C-x h M-w C-x b foo RET C-y.)

> However, as soon as I save this new buffer to a file the bug appears
> again and point moves to random positions in the buffer with C-n/C-f.
> I tried various coding systems for saving the file, but it doesn't
> seem to be relevant.

Does the random movement appear again just by saving that
new buffer, or it appears again after saving and re-reading
from the new file?

And, is that random movement really random; i.e. each time
when you type C-n at the head of HELLO file, point moves to
the different point?

---
Kenichi Handa
handa@ni.aist.go.jp

Re: composition bug

[Romain Francoise]

Kenichi Handa <handa@m17n.org> writes:

> Does the random movement appear again just by saving that
> new buffer, or it appears again after saving and re-reading
> from the new file?

No, just saving the file is enough.

> And, is that random movement really random; i.e. each time
> when you type C-n at the head of HELLO file, point moves to
> the different point?

Good point: no.  If I start from the same place every time then it's
clearly not random, it always goes to the same places in the buffer.

Starting from emacs -Q, then C-h h:
- the first C-n moves to the Arabic character #x633 (point 256 of 3098)
- the next C-n moves to a comma (point 414 of 3098)
- the next C-n moves to Arabic character #x64a (point 759 of 3098)
- then to point 834 (Myanmar character), then 1557 (another comma), etc.

Re: composition bug

[Kenichi Handa]

In article <87zlm86191.fsf@elegiac.orebokech.com>, Romain Francoise <romain@orebokech.com> writes:

> > And, is that random movement really random; i.e. each time
> > when you type C-n at the head of HELLO file, point moves to
> > the different point?

> Good point: no.  If I start from the same place every time then it's
> clearly not random, it always goes to the same places in the buffer.

> Starting from emacs -Q, then C-h h:
> - the first C-n moves to the Arabic character #x633 (point 256 of 3098)
> - the next C-n moves to a comma (point 414 of 3098)
> - the next C-n moves to Arabic character #x64a (point 759 of 3098)
> - then to point 834 (Myanmar character), then 1557 (another comma), etc.

Hmmm, it seems that there's a pattern.  What does this
return when the point is 1, 256, 414:

  ESC : (find-composition (point) (point-max) RET

---
Kenichi Handa
handa@ni.aist.go.jp

Re: composition bug

[Romain Francoise]

Kenichi Handa <handa@m17n.org> writes:

> Hmmm, it seems that there's a pattern.  What does this
> return when the point is 1, 256, 414:

>   ESC : (find-composition (point) (point-max) RET

(255 256 [[#<font-object "-mutt-clearlyu-medium-r-normal--17-120-100-100-p-123-iso10646-1"> 1617] 0 [0 0 1617 1617 0 1 6 15 -12 [0 0 6]]])

(412 414 [[#<font-object "-mutt-clearlyu-medium-r-normal--17-120-100-100-p-123-iso10646-1"> 3732 3765] 1 [0 1 3732 3732 8 1 8 9 0 nil] [0 1 3765 3765 0 -7 2 13 -10 nil]])

(419 420 [[#<font-object "-misc-fixed-medium-r-semicondensed--13-120-75-75-c-60-iso10646-1"> 8205] 2 [0 0 8205 8205 6 0 0 0 0 [0 0 6]]])

Re: composition bug

[Kenichi Handa]

In article <87prn457u2.fsf@elegiac.orebokech.com>, Romain Francoise <romain@orebokech.com> writes:

> Kenichi Handa <handa@m17n.org> writes:
> > Hmmm, it seems that there's a pattern.  What does this
> > return when the point is 1, 256, 414:

> >   ESC : (find-composition (point) (point-max) RET

> (255 256 [[#<font-object "-mutt-clearlyu-medium-r-normal--17-120-100-100-p-123-iso10646-1"> 1617] 0 [0 0 1617 1617 0 1 6 15 -12 [0 0 6]]])

> (412 414 [[#<font-object "-mutt-clearlyu-medium-r-normal--17-120-100-100-p-123-iso10646-1"> 3732 3765] 1 [0 1 3732 3732 8 1 8 9 0 nil] [0 1 3765 3765 0 -7 2 13 -10 nil]])

> (419 420 [[#<font-object "-misc-fixed-medium-r-semicondensed--13-120-75-75-c-60-iso10646-1"> 8205] 2 [0 0 8205 8205 6 0 0 0 0 [0 0 6]]])


Ok, so, C-n at the beginning of HELLO moves the point to the
end of first composition, and the 2nd C-n moves the point to
the end of 2nd composition.  But, the 3rd C-n doesn't
follows that pattern.

Please run Emacs under gdb as below:
          
M-x gdb RET /usr/local/work/emacs/src/emacs RET
            |-----------------------------|-> please adjust for your case
(gdb) br composite.c:1432
(gdb) run -Q

Then visit HELLO file by the Emacs running under gdb, and
type C-n.  Emacs should stop at the break point.  Then,
please find why the point moves to 256 by running the code
one line by line.

---
Kenichi Handa
handa@ni.aist.go.jp

Re: composition bug

[Romain Francoise]

Kenichi Handa <handa@m17n.org> writes:

> (gdb) br composite.c:1432

Thanks, I should have started there because the problem was
immediately obvious when in gdb: an EMACS_INT arg was being
given the literal -1, which is int.  On my machine (amd64),
EMACS_INT is long and due to the traditional function prototype
the value isn't casted automatically.  I installed a fix.

By the way, it looks like the functions in this file use int for
buffer positions in many places, there could be other bugs like this
one.  Building with `-Wtraditional-conversion' on amd64 finds quite
a few occurrences of width mismatch.

Re: composition bug

[Kenichi Handa]

In article <873ajy603u.fsf@elegiac.orebokech.com>, Romain Francoise <romain@orebokech.com> writes:

> Kenichi Handa <handa@m17n.org> writes:
> > (gdb) br composite.c:1432

> Thanks, I should have started there because the problem was
> immediately obvious when in gdb: an EMACS_INT arg was being
> given the literal -1, which is int.  On my machine (amd64),
> EMACS_INT is long and due to the traditional function prototype
> the value isn't casted automatically.  I installed a fix.

Ah!!! Thank you for fixing it.

> By the way, it looks like the functions in this file use int for
> buffer positions in many places, there could be other bugs like this
> one.  Building with `-Wtraditional-conversion' on amd64 finds quite
> a few occurrences of width mismatch.

I'm always confused by int and EMACS_INT.  "struct it" and
"struct text_pos" in dispextern.h uses "int" for
buffer/string positions, and xdisp.c calls functions in
composite.c with those values.  So, there exist mixture of
int and EMACS_INT.  Shouldn't all of them use EMACS_INT for
positions (and perhaps for string length)?

---
Kenichi Handa
handa@ni.aist.go.jp

Re: composition bug

[Stefan Monnier]

> I'm always confused by int and EMACS_INT.  "struct it" and
> "struct text_pos" in dispextern.h uses "int" for
> buffer/string positions, and xdisp.c calls functions in
> composite.c with those values.  So, there exist mixture of
> int and EMACS_INT.  Shouldn't all of them use EMACS_INT for
> positions (and perhaps for string length)?

Yes.  It's important for buffers (or strings) larger than 2GB.
We probably don't need to worry about strings larger than 2GB for a few
more years, but for buffers it's worthwhile to take it into account.


        Stefan

Re: composition bug

[Kenichi Handa]

In article <jwv3ajy7zvr.fsf-monnier+emacs@gnu.org>, Stefan Monnier <monnier@iro.umontreal.ca> writes:

> > I'm always confused by int and EMACS_INT.  "struct it" and
> > "struct text_pos" in dispextern.h uses "int" for
> > buffer/string positions, and xdisp.c calls functions in
> > composite.c with those values.  So, there exist mixture of
> > int and EMACS_INT.  Shouldn't all of them use EMACS_INT for
> > positions (and perhaps for string length)?

> Yes.  It's important for buffers (or strings) larger than 2GB.
> We probably don't need to worry about strings larger than 2GB for a few
> more years, but for buffers it's worthwhile to take it into account.

As I don't have a time to work on it at the moment, I added
this section in admin/FOR-RELEASE.

** In C, use EMACS_INT for variables and structure members
for buffer/string positions.  E.g. struct it, struct text_pos.

---
Kenichi Handa
handa@ni.aist.go.jp

Re: composition bug

[Stefan Monnier]

> As I don't have a time to work on it at the moment, I added
> this section in admin/FOR-RELEASE.

> ** In C, use EMACS_INT for variables and structure members
> for buffer/string positions.  E.g. struct it, struct text_pos.

Yes, it's not urgent, and it can be done incrementally.  As a matter of
fact, it is being done incrementally.  Just keep it in the back of your
head, and whenever you write new code or change old code, switch those
ints to EMACS_INT while you're there.


        Stefan

Re: composition bug

[Kenichi Handa]

In article <jwv3ajw7t3p.fsf-monnier+emacs@gnu.org>, Stefan Monnier <monnier@iro.umontreal.ca> writes:

> Yes, it's not urgent, and it can be done incrementally.  As a matter of
> fact, it is being done incrementally.  Just keep it in the back of your
> head, and whenever you write new code or change old code, switch those
> ints to EMACS_INT while you're there.

Ok.

---
Kenichi Handa
handa@ni.aist.go.jp