Re: Does mozilla allow modification of Strings
On Tue, Jan 29, 2019 at 02:08:59AM -0800, Nanday Dan wrote: On Tuesday, January 29, 2019 at 12:22:08 AM UTC+5:30, Kris Maglione wrote: >Is it possible to add an extra variable to mozilla string(nsTStringRepr). > > >I added a bool variable to nsTStringRepr class in Xpcom/Strings/ As something of a side note, the general way to do this is to add a new data or class flag, and store it in one of the existing fields: I think storing in existing fields will break the actual usage/functioning of those fields and it will make more complex. So I felt like adding a new flag will not affect normal functioning of it. You have this entirely backward. The existing flags fields are meant to store arbitrary boolean flags that describe a string's contents. Existing code knows about them, and is prepared to deal with them. Adding a new field to the string representation, on the other hand, is a drastic change that existing code is *not* prepared to deal with. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Does mozilla allow modification of Strings
On Monday, January 28, 2019 at 6:35:30 PM UTC+5:30, Emilio Cobos Álvarez wrote: > There are probably two different issues. > > On 1/28/19 1:51 PM, wrote: > > > > > > Is it possible to add an extra variable to mozilla string(nsTStringRepr). > > > > > > I added a bool variable to nsTStringRepr class in Xpcom/Strings/ > > > > While building the build I got static Assertions like below > > > > /User/Desktop/MOZB/mozilla-central/xpcom/base/nsCycleCollector.cpp:1954:3: > > error: static_assert failed "Don't create too large CCGraphBuilder objects" > > 0:34.99 static_assert(sizeof(CCGraphBuilder) <= 4096, > > If you're just experimenting this assertion can probably just be ignored > / commented out, seems it's just to prevent accidentally wasting a lot > of memory. > > > > > > > > > In normal execution (without adding any variable to core string )size of > > CCGraphBuilder is 4096 > > > > Since it is static_assert I commented that line and proceeded in > > building. > > > > Building was successful. > > > > While running firefox it just popsup and crashes. > > > > I used debbugger to know where it is getting crashed > > It is at > > > > /mozilla-central/servo/components/style/gecko/data.rs > > 75: debug_assert!(!self.inner().mContents.mRawPtr.is_null()); > > > > I guess it is getting crashed because of memory issue. I don't exactly. > > Does anyone know what is the exact problem and help me in modification of > > mozilla string. > > This one probably means that you forgot to also update the rust version > of the string representation in: > > > https://searchfox.org/mozilla-central/rev/4faab2f1b697827b93e16cb798b22b197e5235c9/xpcom/rust/nsstring/src/lib.rs#368 > > And as a result you're corrupting some memory somewhere. I think we have > unit tests that would catch that mistake: > > > https://searchfox.org/mozilla-central/rev/4faab2f1b697827b93e16cb798b22b197e5235c9/xpcom/rust/nsstring/src/lib.rs#1302 > > If you updated both, then I don't know and I'd have to take a look on a > debugger :) > > -- Emilio Thank you Emilio it worked when I updated the rust string version. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Does mozilla allow modification of Strings
On 1/29/19 4:37 AM, Nanday Dan wrote: I added the variable to FakeString. But still application crashes. Sure, unless you also fixed the Rust interop issue. -Boris ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Does mozilla allow modification of Strings
On Tuesday, January 29, 2019 at 12:22:08 AM UTC+5:30, Kris Maglione wrote: > >Is it possible to add an extra variable to mozilla string(nsTStringRepr). > > > > > >I added a bool variable to nsTStringRepr class in Xpcom/Strings/ > > As something of a side note, the general way to do this is to > add a new data or class flag, and store it in one of the > existing fields: > I think storing in existing fields will break the actual usage/functioning of those fields and it will make more complex. So I felt like adding a new flag will not affect normal functioning of it. > https://searchfox.org/mozilla-central/rev/5c8ea961d04767db723a0a15e3a8f7fbca154129/xpcom/string/nsTStringRepr.h#300-301 > > Changing the size or layout of the string class is more or less > guaranteed to cause problems. Aside from affecting the size and > padding of thousands of classes, the strings API is some of the > oldest code in Gecko, and there is almost certainly some subtle > code which makes assumptions about its layout, and will break if > it changes. The Rust bindings are one such example (as you saw), > but there are almost certainly others as well. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Does mozilla allow modification of Strings
On Monday, January 28, 2019 at 9:09:05 PM UTC+5:30, Boris Zbarsky wrote: > On 1/28/19 10:30 AM, Nanday Dan wrote: > > Sorry, I forgot to mention that I commented those lines too. > > If you comment those lines, you will likely end up with code reading > random values for your boolean at best and crashing at worst (if you > added the boolean at the end; if not, it's just all going to be bad). > You need to make sure that the layout of FakeString exactly matches the > layout of nsString, probably by adding the same boolean to FakeString. > > -Boris I added the variable to FakeString. But still application crashes. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Does mozilla allow modification of Strings
On Mon, Jan 28, 2019 at 04:51:12AM -0800, edusubscr...@gmail.com wrote: Is it possible to add an extra variable to mozilla string(nsTStringRepr). I added a bool variable to nsTStringRepr class in Xpcom/Strings/ As something of a side note, the general way to do this is to add a new data or class flag, and store it in one of the existing fields: https://searchfox.org/mozilla-central/rev/5c8ea961d04767db723a0a15e3a8f7fbca154129/xpcom/string/nsTStringRepr.h#300-301 Changing the size or layout of the string class is more or less guaranteed to cause problems. Aside from affecting the size and padding of thousands of classes, the strings API is some of the oldest code in Gecko, and there is almost certainly some subtle code which makes assumptions about its layout, and will break if it changes. The Rust bindings are one such example (as you saw), but there are almost certainly others as well. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Does mozilla allow modification of Strings
On Mon, Jan 28, 2019 at 02:05:18PM +0100, Emilio Cobos Álvarez wrote: There are probably two different issues. On 1/28/19 1:51 PM, edusubscr...@gmail.com wrote: Is it possible to add an extra variable to mozilla string(nsTStringRepr). I added a bool variable to nsTStringRepr class in Xpcom/Strings/ While building the build I got static Assertions like below /User/Desktop/MOZB/mozilla-central/xpcom/base/nsCycleCollector.cpp:1954:3: error: static_assert failed "Don't create too large CCGraphBuilder objects" 0:34.99 static_assert(sizeof(CCGraphBuilder) <= 4096, If you're just experimenting this assertion can probably just be ignored / commented out, seems it's just to prevent accidentally wasting a lot of memory. Indeed. If the size of that object grows one byte above 4096, each instance takes 8K of (mostly unused) memory rather than 4K. That probably doesn't matter for debugging, but for real world usage, it matters a lot. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Does mozilla allow modification of Strings
On 1/28/19 7:51 AM, edusubscr...@gmail.com wrote: Building was successful. I'm really surprised the static assert at https://searchfox.org/mozilla-central/rev/4faab2f1b697827b93e16cb798b22b197e5235c9/dom/bindings/FakeString.h#146-147 did not trigger, given the changes you described... -Boris ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Does mozilla allow modification of Strings
There are probably two different issues. On 1/28/19 1:51 PM, edusubscr...@gmail.com wrote: > > > Is it possible to add an extra variable to mozilla string(nsTStringRepr). > > > I added a bool variable to nsTStringRepr class in Xpcom/Strings/ > > While building the build I got static Assertions like below > > /User/Desktop/MOZB/mozilla-central/xpcom/base/nsCycleCollector.cpp:1954:3: > error: static_assert failed "Don't create too large CCGraphBuilder objects" > 0:34.99 static_assert(sizeof(CCGraphBuilder) <= 4096, If you're just experimenting this assertion can probably just be ignored / commented out, seems it's just to prevent accidentally wasting a lot of memory. > > > > In normal execution (without adding any variable to core string )size of > CCGraphBuilder is 4096 > > Since it is static_assert I commented that line and proceeded in building. > > Building was successful. > > While running firefox it just popsup and crashes. > > I used debbugger to know where it is getting crashed > It is at > > /mozilla-central/servo/components/style/gecko/data.rs > 75: debug_assert!(!self.inner().mContents.mRawPtr.is_null()); > > I guess it is getting crashed because of memory issue. I don't exactly. > Does anyone know what is the exact problem and help me in modification of > mozilla string. This one probably means that you forgot to also update the rust version of the string representation in: https://searchfox.org/mozilla-central/rev/4faab2f1b697827b93e16cb798b22b197e5235c9/xpcom/rust/nsstring/src/lib.rs#368 And as a result you're corrupting some memory somewhere. I think we have unit tests that would catch that mistake: https://searchfox.org/mozilla-central/rev/4faab2f1b697827b93e16cb798b22b197e5235c9/xpcom/rust/nsstring/src/lib.rs#1302 If you updated both, then I don't know and I'd have to take a look on a debugger :) -- Emilio ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Does mozilla allow modification of Strings
Is it possible to add an extra variable to mozilla string(nsTStringRepr). I added a bool variable to nsTStringRepr class in Xpcom/Strings/ While building the build I got static Assertions like below /User/Desktop/MOZB/mozilla-central/xpcom/base/nsCycleCollector.cpp:1954:3: error: static_assert failed "Don't create too large CCGraphBuilder objects" 0:34.99 static_assert(sizeof(CCGraphBuilder) <= 4096, In normal execution (without adding any variable to core string )size of CCGraphBuilder is 4096 Since it is static_assert I commented that line and proceeded in building. Building was successful. While running firefox it just popsup and crashes. I used debbugger to know where it is getting crashed It is at /mozilla-central/servo/components/style/gecko/data.rs 75: debug_assert!(!self.inner().mContents.mRawPtr.is_null()); I guess it is getting crashed because of memory issue. I don't exactly. Does anyone know what is the exact problem and help me in modification of mozilla string. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
