Re: Testing for expected crashes in both C++ and js, for printf checking
Axel The other easy way to reduce impact here is to reduce the use of Axel nsTextFormatter, or create a replacement that doesn't crash. L20n Axel would be one, or maybe there's C++ template stuff that can taint Axel values with their original types. Tom I don't think there is a compile-time solution to this part of problem, Tom because the substitution is done at runtime. Tom I think what is needed for this problem is a runtime check to verify Tom that the translated format string is consistent with the primary one. I was thinking about this more and I realized you were correct. If the formatter is a variadic template function, then it's possible to ensure at runtime that a particular format option corresponds to the type of the actual argument. This approach would supply runtime safety. Instead of crashing, the formatter could return an error. I think compile-time checking is also worth doing. Maybe it's even possible using some scary combination of constexpr and variadic templates; fun!, though I imagine a compiler plugin is simpler. Tom ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Testing for expected crashes in both C++ and js, for printf checking
Axel The other easy way to reduce impact here is to reduce the use of Axel nsTextFormatter, or create a replacement that doesn't crash. L20n Axel would be one, or maybe there's C++ template stuff that can taint Axel values with their original types. Tom I don't think there is a compile-time solution to this part of problem, Tom because the substitution is done at runtime. Tom I think what is needed for this problem is a runtime check to verify Tom that the translated format string is consistent with the primary one. I was thinking about this more and I realized you were correct. If the formatter is a variadic template function, then it's possible to ensure at runtime that a particular format option corresponds to the type of the actual argument. This approach would supply runtime safety. Instead of crashing, the formatter could return an error. I think compile-time checking is also worth doing. Maybe it's even possible using some scary combination of constexpr and variadic templates; fun!, though I imagine a compiler plugin is simpler. Tom ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Testing for expected crashes in both C++ and js, for printf checking
On Thu, Feb 19, 2015, at 06:32 AM, a...@mozilla.com wrote: On Tuesday, February 17, 2015 at 6:33:42 PM UTC+1, Ted Mielczarek wrote: On Tue, Feb 17, 2015, at 10:27 AM, Axel Hecht wrote: Hi, I'd like to write tests to validate my assumptions around what's an error and what's a warning for localized values going into nsTextFormatter::smprintf. Basically, the tests would start with a reference string, and then a more or less random modification of that string, and a check if the segments are in, or if it crashes [1]. So I'll need a .cpp core, and a wrapper that feeds it data and checks the output. Any suggestions on how to do that right? When you say crashes do you mean an actual program crash? If so, shouldn't we just be fixing these in nsTextFormatter? If you don't need to monitor actual crashes then a Gtest should work just fine here. -Ted I'm talking actual crashes, and I don't know how we would fix the text formatter. I'm glancing at http://mxr.mozilla.org/mozilla-central/source/xpcom/glue/nsTextFormatter.cpp#778, and I had no idea how to fix the C++ API there. bug 1133554#c2 is a bug where we got it wrong in en-US, too. Doesn't crash, but produces undesirable results. Note, folks using the stringbundle.formatStringFromName just use a wstring array. The only code paths affected are those that get the string, and explicitly call into nsTextFormatter from c++. Can you provide an example of a call that crashes? I still feel like the right outcome here is to make the code not crash. -Ted ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Testing for expected crashes in both C++ and js, for printf checking
On Friday, February 20, 2015 at 12:57:44 PM UTC+1, Ted Mielczarek wrote:
On Thu, Feb 19, 2015, at 06:32 AM, a...@mozilla.com wrote:
On Tuesday, February 17, 2015 at 6:33:42 PM UTC+1, Ted Mielczarek wrote:
On Tue, Feb 17, 2015, at 10:27 AM, Axel Hecht wrote:
Hi,
I'd like to write tests to validate my assumptions around what's an
error
and what's a warning for localized values going into
nsTextFormatter::smprintf.
Basically, the tests would start with a reference string, and then a
more
or less random modification of that string, and a check if the segments
are in, or if it crashes [1].
So I'll need a .cpp core, and a wrapper that feeds it data and checks
the
output.
Any suggestions on how to do that right?
When you say crashes do you mean an actual program crash? If so,
shouldn't we just be fixing these in nsTextFormatter? If you don't need
to monitor actual crashes then a Gtest should work just fine here.
-Ted
I'm talking actual crashes, and I don't know how we would fix the text
formatter. I'm glancing at
http://mxr.mozilla.org/mozilla-central/source/xpcom/glue/nsTextFormatter.cpp#778,
and I had no idea how to fix the C++ API there.
bug 1133554#c2 is a bug where we got it wrong in en-US, too. Doesn't
crash, but produces undesirable results.
Note, folks using the stringbundle.formatStringFromName just use a
wstring array. The only code paths affected are those that get the
string, and explicitly call into nsTextFormatter from c++.
Can you provide an example of a call that crashes? I still feel like the
right outcome here is to make the code not crash.
-Ted
I just tweaked the test:
diff --git a/xpcom/tests/TestTextFormatter.cpp
b/xpcom/tests/TestTextFormatter.cpp
--- a/xpcom/tests/TestTextFormatter.cpp
+++ b/xpcom/tests/TestTextFormatter.cpp
@@ -11,7 +11,7 @@
{
int test_ok = true;
- nsAutoString fmt(NS_LITERAL_STRING(%3$s %4$S %1$d %2$d %2$d %3$s));
+ nsAutoString fmt(NS_LITERAL_STRING(%3$s %4$S %1$f %2$d %2$d %3$s));
char utf8[] = Hello;
char16_t ucs2[]={'W', 'o', 'r', 'l', 'd', 0x4e00, 0xAc00, 0xFF45, 0x0103,
0x00};
int d=3;
And then you get
../../dist/bin/TestTextFormatter
Segmentation fault: 11
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
Re: Testing for expected crashes in both C++ and js, for printf checking
Axel == axel-4eJtQOnFJqFBDgjK7y7TUQ axel-4ejtqonfjqfbdgjk7y7...@public.gmane.org writes: Axel We can only do this in the compiler if we actually compiled each Axel localized version by itself. Yes, I see what you mean. Axel The other easy way to reduce impact here is to reduce the use of Axel nsTextFormatter, or create a replacement that doesn't crash. L20n Axel would be one, or maybe there's C++ template stuff that can taint Axel values with their original types. I don't think there is a compile-time solution to this part of problem, because the substitution is done at runtime. I think what is needed for this problem is a runtime check to verify that the translated format string is consistent with the primary one. Tom ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Testing for expected crashes in both C++ and js, for printf checking
On Friday, February 20, 2015 at 6:34:07 PM UTC+1, Tom Tromey wrote: Axel == axel-4eJtQOnFJqFBDgjK7y7TUQ axel-4ejtqonfjqfbdgjk7y7...@public.gmane.org writes: Axel We can only do this in the compiler if we actually compiled each Axel localized version by itself. Yes, I see what you mean. Axel The other easy way to reduce impact here is to reduce the use of Axel nsTextFormatter, or create a replacement that doesn't crash. L20n Axel would be one, or maybe there's C++ template stuff that can taint Axel values with their original types. I don't think there is a compile-time solution to this part of problem, because the substitution is done at runtime. I think what is needed for this problem is a runtime check to verify that the translated format string is consistent with the primary one. Tom I do this to validate the thinking that I've applied to compare-locales, which does source-based checks of localizations against en-US. With that tool, I can strip bad strings during repackaging or language pack building. That doesn't solve all problems, but gets use 90% there. Axel ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Testing for expected crashes in both C++ and js, for printf checking
On Tuesday, February 17, 2015 at 6:33:42 PM UTC+1, Ted Mielczarek wrote: On Tue, Feb 17, 2015, at 10:27 AM, Axel Hecht wrote: Hi, I'd like to write tests to validate my assumptions around what's an error and what's a warning for localized values going into nsTextFormatter::smprintf. Basically, the tests would start with a reference string, and then a more or less random modification of that string, and a check if the segments are in, or if it crashes [1]. So I'll need a .cpp core, and a wrapper that feeds it data and checks the output. Any suggestions on how to do that right? When you say crashes do you mean an actual program crash? If so, shouldn't we just be fixing these in nsTextFormatter? If you don't need to monitor actual crashes then a Gtest should work just fine here. -Ted I'm talking actual crashes, and I don't know how we would fix the text formatter. I'm glancing at http://mxr.mozilla.org/mozilla-central/source/xpcom/glue/nsTextFormatter.cpp#778, and I had no idea how to fix the C++ API there. bug 1133554#c2 is a bug where we got it wrong in en-US, too. Doesn't crash, but produces undesirable results. Note, folks using the stringbundle.formatStringFromName just use a wstring array. The only code paths affected are those that get the string, and explicitly call into nsTextFormatter from c++. Axel ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Testing for expected crashes in both C++ and js, for printf checking
Axel == axel-4eJtQOnFJqFBDgjK7y7TUQ axel-4ejtqonfjqfbdgjk7y7...@public.gmane.org writes: Axel I'm talking actual crashes, and I don't know how we would fix the Axel text formatter. I'm glancing at Axel http://mxr.mozilla.org/mozilla-central/source/xpcom/glue/nsTextFormatter.cpp#778, Axel and I had no idea how to fix the C++ API there. Axel bug 1133554#c2 is a bug where we got it wrong in en-US, too. Doesn't Axel crash, but produces undesirable results. Axel Note, folks using the stringbundle.formatStringFromName just use a Axel wstring array. The only code paths affected are those that get the Axel string, and explicitly call into nsTextFormatter from c++. I think the best answer for C++ is to write a compiler plugin to do format checking. GCC already has code to check that a format string matches the arguments. However, it can't be applied in several places in the tree because: * Various printf-likes in the tree extend the list of formats. For example, JS uses %hs to print a char16_t*. (And nsTextFormatter uses %S for this same thing, oops.) * The in-tree printf-likes sometimes use the same format characters as printf to mean different things. For example, nspr uses %l to mean 32-bit, whereas printf uses it to mean sizeof(long). * Some printf-likes, like nsTextFormatter, use a char16_t* as the format string. There are GCC bugs open about all of these. There's also the issue of looking up the format string in a message catalog. GCC has support for this, too, but I don't know offhand whether it can handle the approach used here. It's possible to fix all these things in the compilers. The advantage of a plugin is that nobody has to wait for a compiler upgrade to use it to find the bugs. I think it's worth doing. Before I learned about the various problems, I took a stab at using the GCC format attributes, and amidst all the nit-picking there are some genuine bugs. Tom ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Testing for expected crashes in both C++ and js, for printf checking
On Thursday, February 19, 2015 at 11:27:11 PM UTC+1, Tom Tromey wrote: Axel writes: Axel I'm talking actual crashes, and I don't know how we would fix the Axel text formatter. I'm glancing at Axel http://mxr.mozilla.org/mozilla-central/source/xpcom/glue/nsTextFormatter.cpp#778, Axel and I had no idea how to fix the C++ API there. Axel bug 1133554#c2 is a bug where we got it wrong in en-US, too. Doesn't Axel crash, but produces undesirable results. Axel Note, folks using the stringbundle.formatStringFromName just use a Axel wstring array. The only code paths affected are those that get the Axel string, and explicitly call into nsTextFormatter from c++. I think the best answer for C++ is to write a compiler plugin to do format checking. GCC already has code to check that a format string matches the arguments. However, it can't be applied in several places in the tree because: * Various printf-likes in the tree extend the list of formats. For example, JS uses %hs to print a char16_t*. (And nsTextFormatter uses %S for this same thing, oops.) * The in-tree printf-likes sometimes use the same format characters as printf to mean different things. For example, nspr uses %l to mean 32-bit, whereas printf uses it to mean sizeof(long). * Some printf-likes, like nsTextFormatter, use a char16_t* as the format string. There are GCC bugs open about all of these. There's also the issue of looking up the format string in a message catalog. GCC has support for this, too, but I don't know offhand whether it can handle the approach used here. It's possible to fix all these things in the compilers. The advantage of a plugin is that nobody has to wait for a compiler upgrade to use it to find the bugs. I think it's worth doing. Before I learned about the various problems, I took a stab at using the GCC format attributes, and amidst all the nit-picking there are some genuine bugs. Tom We can only do this in the compiler if we actually compiled each localized version by itself. As long as we're using runtime files to switch languages, we can't rely on the compiler to check these things. There are two facets of l10n that we would loose if we went for a compiler-based solution: - language packs, i.e., addons that allow to install just another language. - repacks to create a localized build. This is mostly good use of resources, given that a repack takes about a minute or two per language. The other easy way to reduce impact here is to reduce the use of nsTextFormatter, or create a replacement that doesn't crash. L20n would be one, or maybe there's C++ template stuff that can taint values with their original types. Axel (PS: I wonder why I can't post to .platform via NNTP anymore :-/ ) ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Testing for expected crashes in both C++ and js, for printf checking
On Tue, Feb 17, 2015, at 10:27 AM, Axel Hecht wrote: Hi, I'd like to write tests to validate my assumptions around what's an error and what's a warning for localized values going into nsTextFormatter::smprintf. Basically, the tests would start with a reference string, and then a more or less random modification of that string, and a check if the segments are in, or if it crashes [1]. So I'll need a .cpp core, and a wrapper that feeds it data and checks the output. Any suggestions on how to do that right? When you say crashes do you mean an actual program crash? If so, shouldn't we just be fixing these in nsTextFormatter? If you don't need to monitor actual crashes then a Gtest should work just fine here. -Ted ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
