[espeak-ng:master] reported: The change to tests/translate.test to detect #824 causes unexplained failures on some builds #github


espeak-ng@groups.io Integration <espeak-ng@...>
 

[espeak-ng:master] New Comment on Issue #945 The change to tests/translate.test to detect #824 causes unexplained failures on some builds
By jaacoppi:

I think you are trying to fix the wrong problem.

The test case for #824 is to prevent espeak-ng from crashing with long strings. It is not specific to Russian, it also happens with other languages. Whether or not the pronunciation is correct is not important.

I think fixing the test for certain compilers requires identifying which compilers fail and figuring out what has changed in the compiler. The ynthesizer should not produce different results with different compilers.

See: https://github.com/espeak-ng/espeak-ng/issues/824 https://github.com/espeak-ng/espeak-ng/pull/867


espeak-ng@groups.io Integration <espeak-ng@...>
 

[espeak-ng:master] New Comment on Issue #945 The change to tests/translate.test to detect #824 causes unexplained failures on some builds
By jbowler:

Please give me an example of a compiler which, with the current HEAD, produces an espeak-ng that does not treat the periods as end-of-sentence (i.e where make tests/translate.check passes). E.g. what do you use, what does @rhdunn use...


espeak-ng@groups.io Integration <espeak-ng@...>
 

[espeak-ng:master] New Comment on Issue #945 The change to tests/translate.test to detect #824 causes unexplained failures on some builds
By rhdunn:

Can you run the test through -X to see what rules it is considering/using to get the pronunciation you are seeing? From that, we could run and compare the output.

Note: It is common to add tests for issues, fix them, then close the issues. That way, you don't get any regressions of that bug in the future when making changes to the code. That is why #824 is closed as that is about a crash, not a difference in the output.


espeak-ng@groups.io Integration <espeak-ng@...>
 

[espeak-ng:master] New Comment on Issue #945 The change to tests/translate.test to detect #824 causes unexplained failures on some builds
By jbowler:

The ssml problem is #947. My clangs only run back to clang-10 and gccs to gcc-6.5.0 (which, for some reason, is masked by Gentoo). I do have 8.4.0 - that's actually the earliest unmasked version I have. Anyway, my current reduced test string is "0.8", which captures the first occurence of the a -> a# phoneme change. The phonemes reported for the translation are as in the test_phon call:

Found: '_.' [t'otS;ka]
Found: '_0' [n'ojl;]
Found: '_8' [v'os;E2m]
n'ojl t'otS;ka# v'os;E2m

That's from under gdb so I'll look at where the transmutation is coming from.


espeak-ng@groups.io Integration <espeak-ng@...>
 

[espeak-ng:master] New Comment on Issue #945 The change to tests/translate.test to detect #824 causes unexplained failures on some builds
By rhdunn:

I get the same with that example:

$ src/espeak-ng -v ru -Xq 0.8
Found: '_.' [t'otS;ka]
Found: '_0' [n'ojl;]
Found: '_8' [v'os;E2m]
n'ojl t'otS;ka# v'os;E2m

It would be helpful to do that with the whole input in the test so we can compare before trying to minimize the example. -- Specifically, the a => a# transformation is likely to be in the phoneme rules with adjacent phonemes, and not the actual punctuation. It is more likely that one of the variables that is used in the logic to process the phoneme transformation rules is used after being unset (or something similar) and the memory in that position is handled differently with the compiler you have.


espeak-ng@groups.io Integration <espeak-ng@...>
 

[espeak-ng:master] New Comment on Issue #945 The change to tests/translate.test to detect #824 causes unexplained failures on some builds
By jbowler:

It's happening because the a phoneme at the end of the "." [t'otS;ka] is followed by a phoneme v at the start of "8" [v'os;E2m] and this (v) phoneme has PHONEME_LIST2::sourceix set to a non-zero value 2051 [0x803] ('only set at the start of a word'). This causes "word_end" to be set to true in SubstituePhonemes and this means that first test (replace_flags & 1) in the replace_phonemes loop is not skipped.

Anyway, the "." is not punctuation - it's being spoken - there are three words there, "nought period eight" in en-gb and this is reflected in both the original and new phoneme streams: in both cases the "a" phoneme is followed by a space in the output. I believe that means that there is a word-end or maybe a pause.

phsource/ph_russian has this for "phoneme a":

IF thisPh(isWordEnd) THEN FMT(vowel/a#_3) ENDIF

And I believe that is what results in the substitution, anyway you added it on 5/15/2018:

5c6a4c880 (Reece H. Dunn 2018-05-15 07:22:29 +0100 498) FMT(vowel/a#_3)

This is what the (sole) entry in replace_phonemes does - it takes the 'a' phoneme and replaces it by 'a#'.

I don't know if the ph_russian change was meant to result in that substitution, but it certainly seems to be causing it. It seems odd to me that it is FMT(), not ChangePhoneme() as above but I haven't traced the building of replace_phonemes yet.


espeak-ng@groups.io Integration <espeak-ng@...>
 

[espeak-ng:master] New Comment on Issue #945 The change to tests/translate.test to detect #824 causes unexplained failures on some builds
By rhdunn:

I only made the indentation for that line consistent if you looked at the 5c6a4c880 commit ("ru: format the ph_russian file and remove commented out lines"):

   IF thisPh(isWordEnd) THEN
-       FMT(vowel/a#_3)
+    FMT(vowel/a#_3)
   ENDIF

The change was introduced in 20f52f03012d64873ab79bb7693bfadbd376d144 as part of the espeak 1.42.21 release by Johnathan Duddington.

You said above "In both cases the "." is a sentence terminator." and noted in issue #947 > Note that this seems related to #945; in both cases an incorrectly formatted piece of text results from spurious (#945) or missing (#947) sentence terminators."

Hence me referring to it as "punctuation". -- For things like 0.2 the . is actually being handled by the number logic so is a decimal/fraction part divider, hence it using: > Found: '_.' [t'otS;ka]

That test is designed deliberately to use a long word, which resulted in a crash previously. The espeak engine has a fixed word length it can process, after which it splits the word into two parts. If that logic has an out-of-bounds read issue that could cause the issue you are seeing if the compiler has changed the way it is reading data at that point, or compiler flags the program is built with.

What output do you get when you run that failing test with -X, i.e.: > src/espeak-ng -v ru -Xq "C0.87569253%200%200%201%200.36817%2C-0.2019%201.0387963%2C1.0387963%200%200%201%200.419637%2C-0.02375%20c%200.30087%2C0.04514%200.582739%2C0.216942%200.750593%2C0.470308%200.09502%2C0.142517%200.153603%2C0.308788%200.18844%2C0.478226%200.03484%2C0.168646%200.0475%2C0.340459%200.05701%2C0.513064%200.03167%2C0.601741%200.03167%2C1.205067%200.01426%2C1.808392%20-0.01426%2C0.526524%20-0.04355%2C1.0673%20-0.253366%2C1.549486%20-0.271575%2C0.619159%20-0.817101%2C1.08155%20-1.405383%2C1.414092%20a%205.5835296%2C5.5835296%200%200%201%20-1.257323%2C0.512272%20c%200.38163%2C1.219319%200.580363%2C2.56532%200.580363%2C3.93349%20a%2013.935071%2C13.935071%200%200%201%20-0.106901%2C1.682498%2010.264446%2C10.264446%200%200%200%205.054631%2C-8.829768%20c%200%2C-5.669041%20-4.59699%2C-10.2652391%20-10.265238%2C-10.2652391%20z%20M%2010.037865%2C19.798537%20v%201.174188%20a%201.488519%2C1.488519%200%200%201%200.857482%2C0.286619%201.3760882%2C1.3760882%200%200%201%200.440222%2C0.538402%20c%200.0966%2C0.213775%200.131432%2C0.456056%200.09184%2C0.687252%20a%201.1821057%2C1.1821057%200%200%201%20-0.262867%2C0.560568%201.3040376%2C1.3040376%200%200%201%20-0.502772%2C0.36263%201.3760882%2C1.3760882%200%200%201%20-0.623119%2C0.0966%20v%202.953287%20l%206.145683%2C-3.33175%20-6.145683%2C-3"


espeak-ng@groups.io Integration <espeak-ng@...>
 

[espeak-ng:master] New Comment on Issue #945 The change to tests/translate.test to detect #824 causes unexplained failures on some builds
By jbowler:

"8.1" is not a Russian number. Put a breakpoint on "TranslateClause", "TranslateWord2" and "TranslateNumber" then run:

run -Xq -v -ru "0.8"

You will see that TranslateClause is called once and TranslateWord2 twice, with each call resulting in a call to TranslateNumber. One or other of those swallows the "." somehow, probably the second because "." is not valid in a (Russian) number; there are two numbers with an intervening spoken period [t'otS;ka].

Now try with a real number:

espeak-ng -Xq -v ru "0,8"

TranslateClause, Word2 and Number are called only once and the output is:

Found: '_dpt' [_:'i_:]
Found: '_0' [n'ojl;]
Found: '_8' [v'os;E2m]
Found: '_dpt2' [_d;Is;'atYx]
n'ojl _:'i_: v'os;E2m_d;Is;'AtYx

Notice that the period has gone but there is no comma (is that even punctuation in Russian?) Instead the Russian for start/end of a decimal fraction are output. The language seems to speak something like, "nought decimal eight-end". I can't speak Russian so I don't know, but when I use Google Translate it certainly sounds like the equivalent of those three words. It's not possible to get Google Translate to speak "0.8" because it isn't Russian...

The output from the test is as described before - every "a " sequence in the original test phoneme stream becomes "a# ". I've attached a .gx .out Xq.out.gz file with the full -X output.

The original test string is, of course, gibberish - it looks like it is a fragment of an XML file or maybe a URI and it starts with a "C" which I assume is from a %2C in the original. It seems to me a better way to test not crashing on garbage input would be to just generate a random byte stream, but it should really be covered by the fuzzer tests.


espeak-ng@groups.io Integration <espeak-ng@...>
 

[espeak-ng:master] New Comment on Issue #945 The change to tests/translate.test to detect #824 causes unexplained failures on some builds
By rhdunn:

I was wrong w.r.t. the reading of fractions. The _. entry is for pronouncing the . character. The _dpt and _dpt2 are for the start and end of the decimal fraction.

The point of this test is not to test the number handling (that would be in tests/language-numbers-(cardinal|ordinal).test). The point of translate.test is to check the TranslateClause and related functions (including to some part ReadClause).

BTW this happens with -O0 too, so it doesn't look likely to be either an out-of-bounds or uninitialized memory issue or, for that matter, a compiler issue.

If it is neither a compiler issue nor undefined behaviour (out-of-bounds, etc.), I would expect espeak-ng to use a# on all systems when given the same input. Specifically, why does my system produce a# in 0.8 but not in the test case, whereas yours produces a# on both? -- That is clearly indicating that something is different.

The only way -- per your analysis -- for the test input text is that word_end is true on your machine and false on my machine and the Travis builds. Note: I fixed a bug in a different project that only caused test errors on i386 build machines -- the issue there turned out to be it reading the character after the data string which, when it was a certain character, was causing some logic in the application to be run resulting in different output.

The espeak data such as the word being read and sourceix are arrays stored on the stack -- this is done to keep the application compact on constrained memory devices. If later versions of gcc and clang have changed the stack layout, or add out-of-bounds write sentinels, that could affect what espeak-ng sees when processing the long string (and not when processing shorter strings), and thus the difference in output as noted in the test.

This all indicates that -- again per your analysis -- that it should be outputting a#. What is not clear -- and is what we are trying to isolate and identify -- is why that is not happening on the systems running the tests.

My clangs only run back to clang-10 and gccs to gcc-6.5.0 (which, for some reason, is masked by Gentoo). I do have 8.4.0 - that's actually the earliest unmasked version I have.

You haven't said which compiler and version you are using. If we know that, we can try that compiler/version on our machines to see if we can reproduce the issue.

It seems to me a better way to test not crashing on garbage input would be to just generate a random byte stream, but it should really be covered by the fuzzer tests.

The point of things like generating random byte streams or using fuzzers is to identify new bugs. Once you have identified a bug with a fuzzer, you should add that as a test case to your application, to prevent that bug occurring again.

As has been said, this bug was identified as a crash from a user using NVDA (#824). As such -- in order to prevent a regression -- this has been added to the tests.


espeak-ng@groups.io Integration <espeak-ng@...>
 

[espeak-ng:master] New Comment on Issue #945 The change to tests/translate.test to detect #824 causes unexplained failures on some builds
By jbowler:

Sorry; the compiler/OS info was in #947:

  • gcc (Gentoo 11.1.0 p1) 11.1.0
  • Linux hippopopus 5.12.5-gentoo #1 SMP Thu May 20 09:29:16 PDT 2021 x86_64 Intel(R) Core(TM) i7-8700K CPU @ 3.70GHz GenuineIntel GNU/Linux
  • The OS is Gentoo running the KDE Plasma 17.1 desktop profile: /usr/portage/profiles/default/linux/amd64/17.1/desktop/plasma
  • The audio is: AUDIO_USE="jack pulseaudio openal"

I've got 8.4.0 of gcc installed now, but haven't tried that yet.


espeak-ng@groups.io Integration <espeak-ng@...>
 

[espeak-ng:master] New Comment on Issue #945 The change to tests/translate.test to detect #824 causes unexplained failures on some builds
By jbowler:

Ok, with this configure:

CC=gcc-8.4.0 ./configure --prefix=$HOME/src/espeak-ng/install-8.4.0

and this clone:

~/src/espeak-ng/git-8.4.0 $ git status
HEAD detached at f6d092a9
nothing to commit, working tree clean

I get this:

configure:

    Configuration for eSpeak NG complete.

        Source code location:          .

        C99 Compiler:                  gcc-8.4.0
        C99 Compiler flags:            -Wunused-parameter -Wunused -Wuninitialized -Wreturn-type -Wmissing-prototypes -Wimplicit -g -O2 -std=c99

        Sonic:                         no
        PCAudioLib:                    no

        gradle (Android):              gradle
        ndk-build (Android):           

        Klatt:                         yes
        speechPlayer:                  yes
        MBROLA:                        yes
        Async:                         yes

        Extended Dictionaries:
            Russian:                   no
            Chinese (Mandarin):        no
            Chinese (Cantonese):       no

And then "make check" produces the attached log file, which repros your 8.3.0 set; i.e. testing ru ru sum strings passes. So that gives me something to debug :-) make-check.log


espeak-ng@groups.io Integration <espeak-ng@...>
 

[espeak-ng:master] New Comment on Issue #945 The change to tests/translate.test to detect #824 causes unexplained failures on some builds
By jbowler:

And, indeed; I did a "make install" (to install-8.4.0, as configured) and ran bin/espeak-ng; the test script has the missing '#' characters, the simplified test does not. Swapping to my HEAD test install shows the other behavior; the only change is that my HEAD contains 88c4a452fe40413a146d21e31856fcfa9e7e4eea but it's difficult to believe that this (addition of a sed command in the test) would make a difference to the behavior of an installed espeak-ng...


espeak-ng@groups.io Integration <espeak-ng@...>
 

[espeak-ng:master] New Comment on Issue #945 The change to tests/translate.test to detect #824 causes unexplained failures on some builds
By jbowler:

In 8.4.0 a breakpoint at SubstitutePhonemes shows that replace_phonemes[0] is badly formed. Here's the 11.1.0 version which does the substitute of a -> a#;

(gdb) print replace_phonemes[0]
$2 = {old_ph = 35 '#', new_ph = 136 '\210', type = 3 '\003'}

35 is the phcode of the Russian a phoneme and all the fields seem valid to me. Here is what 8.4.0 has (BTW, this behavior is visible in -O0, so it's unlikely to be an optimization issue):

(gdb) print replace_phonemes[0]
$2 = {old_ph = 32 ' ', new_ph = 0 '\000', type = 21 '\025'}

n_replace_phonemes is still 1. I'm guessing this is a memory overwrite; what gets overwritten often depends on the compiler. replace_phonemes is a global (translate.c/phonemes.h) so a memory overwrite (if that is what it is) is not as bad as a stack overwrite and it shouldn't be hard to track down the preceding structure so check.


espeak-ng@groups.io Integration <espeak-ng@...>
 

[espeak-ng:master] New Comment on Issue #945 The change to tests/translate.test to detect #824 causes unexplained failures on some builds
By jbowler:

In 8.4.0 a breakpoint at SubstitutePhonemes shows that replace_phonemes[0] is badly formed. Here's the 11.1.0 version which does the substitute of a -> a#;

(gdb) print replace_phonemes[0]
$2 = {old_ph = 35 '#', new_ph = 136 '\210', type = 3 '\003'}

35 is the phcode of the Russian a phoneme and all the fields seem valid to me. Here is what 8.4.0 has (BTW, this behavior is visible in -O0, so it's unlikely to be an optimization issue):

(gdb) print replace_phonemes[0]
$2 = {old_ph = 32 ' ', new_ph = 0 '\000', type = 21 '\025'}

n_replace_phonemes is still 1. I'm guessing this is a memory overwrite; what gets overwritten often depends on the compiler. replace_phonemes is a global (translate.c/phonemes.h) so a memory overwrite (if that is what it is) is not as bad as a stack overwrite and it shouldn't be hard to track down the preceding structure so check.

A quick check shows that replace_phonemes with 11.1.0 is uninitialized (all zero) apart from the first element, whereas replace_phonemes with 8.4.0 has been written to up to the 10th element:

(gdb) print /x *(int*)&replace_phonemes[0].old_ph@32
$37 = {0x150020, 0xb000000, 0x150020, 0xb000000, 0x150020, 0x2b000000, 0x90000, 
  0x229, 0x90000, 0x229, 0x0 <repeats 22 times>}


espeak-ng@groups.io Integration <espeak-ng@...>
 

[espeak-ng:master] New Comment on Issue #945 The change to tests/translate.test to detect #824 causes unexplained failures on some builds
By jbowler:

The problem is a write-beyond-end of ph_list2. This contains 1000 elements and with both 8.4.0 and 11.1.0 the code seems to write beyond the end. The difference is that the list is followed by the global count_words in 8.4.0 whereas in 11.1.0 it is followed by option_punctlist. There does seem to be code to truncate the processing; the string in question is generating many more phonemes than there are characters/bytes in the input, but it doesn't seem to be quite correct. With 8.4.0 the overwrite extends into the 'replace_phonemes' global, with 11.1.0 it stops before that.

Here are copies of two sets of debug output obtained at a breakpoint on SubstitutePhonemes, first with 11.1.0 (which, I believe, is producing the correct output despite the overwrite):

(gdb) run -xq -v ru -f 
[fragment.txt](https://github.com/espeak-ng/espeak-ng/files/6528761/fragment.txt)

Starting program: /home/jbowler/src/espeak-ng/install/bin/espeak-ng -xq -v ru -f /tmp/fragment.txt
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".
[New Thread 0x7ffff74bb640 (LWP 13887)]

Thread 1 "espeak-ng" hit Breakpoint 1, SubstitutePhonemes (
    plist_out=0x7fffffff1b00) at src/libespeak-ng/phonemelist.c:56
56              int n_plist_out = 0;
(gdb) print n_ph_list2
$33 = 998
(gdb) print ph_list2[1000]
$34 = {synthflags = 0, phcode = 21 '\025', stresslevel = 0 '\000', 
  sourceix = 0, wordstress = 0 '\000', tone_ph = 11 '\v'}
(gdb) print ph_list2+1000
$35 = (PHONEME_LIST2 *) 0x7ffff7fba3e0 <option_punctlist>
(gdb) print option_punctlist
$36 = L"\x150000\xb000000\x150000\xb000000\x150000\xb000000\x150000\xb000000\x150000\xb000000\x150000\xb000000\x150000\xb000000\x150000\xb000000\x150000\xb000000\x150000\xb000000\x150000\xb000000\x150000\xb000000\x150000\xb000000\x150000\xb000000\x150000\x2b000000\x90000˞\x90000˞", '\000' <repeats 25 times>
(gdb) print count_words
$37 = 232

With 8.4.0, however, we can see that "option_punctlist" should be all zero (uninitialized/unwritten) but that count_words has been damaged:

(gdb) run -xq -v ru -f /tmp/fragment.txt
Starting program: /home/jbowler/src/espeak-ng/install-8.4.0-debug/bin/espeak-ng -xq -v ru -f /tmp/fragment.txt
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".
[New Thread 0x7ffff74bc640 (LWP 13879)]

Thread 1 "espeak-ng" hit Breakpoint 1, SubstitutePhonemes (
    plist_out=0x7fffffff1ac0) at src/libespeak-ng/phonemelist.c:56
56              int n_plist_out = 0;
(gdb) print n_ph_list2
$67 = 1000
(gdb) print ph_list2[1000]
$68 = {synthflags = 126, phcode = 21 '\025', stresslevel = 0 '\000', 
  sourceix = 0, wordstress = 0 '\000', tone_ph = 11 '\v'}
(gdb) print ph_list2+1000
$69 = (PHONEME_LIST2 *) 0x7ffff7fbe820 <count_words>
(gdb) print option_punctlist
$70 = L'\000' <repeats 59 times>
(gdb) print count_words
$71 = 1376382
(gdb) print /x count_words
$72 = 0x15007e

I experimented with English and a file which has 314 bytes (though slightly fewer characters) which expand to slightly more than 998 phonemes; equation.txt. It does not show the problem; both builds truncate the output at 998 phonemes. Likewise if I translate that file into Russian ru-eq.txt (Google Translate) except that in that case the output isn't truncated, it is split.

I increase N_PHONEME_LIST to 10000, which avoids the need to split the string, and got fragment.out.txt; I'm pretty sure this is the correct string however in scenarios where the bug might show the string should be split into two TranslateClause outputs (they are separated by new line).

Anyway, without the increase, the first overwrite is at line 1226 of translate.c inside SetPlist2

(gdb) print p
$12 = (PHONEME_LIST2 *) 0x7ffff7fba3e0 <option_punctlist>

Someone who understands how the truncation/splitting of overlong input is meant to work probably needs to look at it now. It's easy to trap on more recent gcc's because option_punctlist is not actually modified, so a simple "watch" catches the problem (and it's a hardware watch, so it is fast). Catching it with 8.x or earlier is more of a problem since word-count changes; I'll see if I can trap it with (char*)word_count+2 (since word_count shouldn't go over 65535).


espeak-ng@groups.io Integration <espeak-ng@...>
 

[espeak-ng:master] New Comment on Issue #945 The change to tests/translate.test to detect #824 causes unexplained failures on some builds
By jbowler:

It seems to have happening because of all the Latin "C" characters; they switch the phoneme table. There are checks in TranslateWord2 for overflow of ph_list2 in some places but not all. Places that do check check for space for four phonemes, but when the space runs out the code just drops through to more code that assumes there is space for four more phonemes. I suspect the "expected.txt" is actually heavily truncated as a result - there's no proper error recovery if the buffer space runs out, the code just keeps on skipping some phonemes and writing others.


espeak-ng@groups.io Integration <espeak-ng@...>
 

[espeak-ng:master] New Comment on Issue #945 The change to tests/translate.test to detect #824 causes unexplained failures on some builds
By jbowler:

The attached patch does not protect against malware (crackers) but it should detect the overwrite with moderate to good reliability. translate.c.patch.txt

Some manner of malware protection could be achieved using a random byte for ucheck, though not much because the damage would already be done...


espeak-ng@groups.io Integration <espeak-ng@...>
 

[espeak-ng:master] New Comment on Issue #945 The change to tests/translate.test to detect #824 causes unexplained failures on some builds
By jbowler:

I pushed a temporary fix. Please check the pull request.

Verified against 8.4.0; make all (-O0), make check. I've pulled to 11.1.0 and verifed ; fine there too.


espeak-ng@groups.io Integration <espeak-ng@...>
 

[espeak-ng:master] New Comment on Issue #945 The change to tests/translate.test to detect #824 causes unexplained failures on some builds
By rhdunn:

The word buffer size and phoneme list being of comparable size (800 for the source size and 1000 for the phoneme list size) is predicated on the assumption that the number of phonemes will never be longer than the number of characters. This only really works if the word doesn't contain a lot of numbers or emoji that significantly increase the phoneme count compared to the word/character count.

Some things we could do: 1. Change the word splitting/detection algorithm to handle this case better -- e.g. when a - is followed by a number. 2. Flush the phonemes upto the previous word (so it is still affected by the next word) when it reaches a threshold (e.g. within 100 of the last phonemes) -- that is, set the word separator ph value to null (end of phonemes); call Generate(); copy the remaining phonemes (including the space) to the start of the phoneme list. 3. Make phoneme_list a circular buffer -- this would allow us to avoid the copying to shift the phonemes to the start of the list -- it would be worth doing a performance check on the result to see if this doesn't adversely affect performance. 4. Make phoneme_list a dynamic array (malloc/calloc, using realloc/reallocarray to resize, where on failure (e.g. ENOMEM) the original buffer should be unchanged and an espeak errno error should be returned from the API call). -- this should be conditional to support memory-constrained devices like riscos, and enabled when compiling for one of those platforms.

I would start by creating an API around the phoneme list -- e.g. phoneme_list_add, phoneme_list_current, phoneme_list_get, phoneme_list_next, etc. (which should be added based on how the phoneme_list is used) -- That way, we can add guards and/or change it to a circular/dynamic buffer more easily.


espeak-ng@groups.io Integration <espeak-ng@...>
 

[espeak-ng:master] New Comment on Issue #945 The change to tests/translate.test to detect #824 causes unexplained failures on some builds
By jbowler:

The fundamental problem is that the code in TranslateWord2 is not reliably checking for end-of-output-buffer. Unless you do that the only suggestion above that works is (4). Even then there has to be code that, somehow, checks for the need for a new allocation - even with (4) each time a phoneme is added to the end of the list some code has to check for a new allocation. So I'm agreeing with your last paragraph but I think (3) offers some possibilities that maybe avoid this.

Suggestion (3) is easy and safe; it doesn't modify any code and it reliably detects overflow but it has no recovery. In the test case and in my equation.txt a circular buffer would get overwritten in the absence of some form of checking. Nevertheless it's an ingenious suggestion that has the merit that it self-evidently avoids the overflow (so it removes the exploit).

If (2) can be done and you combine it with (3) that might work. I think something like the test case would still fail - in the test case the "words" are ridiculously long because of the paucity of space (ASCII 0x20) characters which are the word separators. That said (2) seems attractive because, why wait to produce output? The current code does seem to search backward from end-of-clause to handle stress, but is that really necessary? How many phoneme look-ahead is really required before any given phoneme is output. Arabic might be a problem; IRC it is necessary to see the whole word to know how to select the glyphs, I don't know if that applies to phonemes too.

I don't understand (1); that's an ASCII \055 hyphen yet "-1" is a number and there is no pause after the "-" in English, whereas "2-1" is three words and there are two pauses. "+1" is also a number, "2-+1" and so on are valid. The rules in other languages may be completely different and actually speaking equations, even simple linear arithmetic, is effectively another language.

I suspect you are fine with a simplistic number parser that just recognizes decimal fractions. If someone wants to work out how to pronounce an engineering or scientific format number, e.g. "1.23E6", or something more mathematical using base 10 superscripts "1.23×106" that strikes me as much bigger and separate thing. My equation.txt example shows that mathematical/computer usages such as the word "iff" (pronounced "if and only if" ;-) and ">number" or "<number" are not currently handled.

If espeak-ng was using a simple circular buffer of size 1024 (i.e. slightly larger than at present) it wouldn't suffer the overwrite and it wouldn't fail in any case where it doesn't at present. I think I could relatively easily submit a patch for this without adding a complete class-based implementation of PHONEME_LIST2. (2) requires changes I don't understand but sounds simple, potentially it would allow ph_list2 to be reduced to, say, 128 phonemes; antidisestablishmentarianism only has 12 phonemes ;-)