#1 2018-12-23 13:50:58

mpv
Member
From: Ukraine
Registered: 2012-03-24
Posts: 1,534
Website

Potential AV in _JsonFast

I'm randomly (very rarely, maybe once out of 100,000 calls, but this is very critical) got an AV
After digging deep into code I'm 90% sure this is because of this code

var
  data: variant;
..
  if fUserDataJSON = '' then
    fUserDataJSON := '{}';
  data := _JsonFast(fUserDataJSON); 

but the actual problem is inside JSON variants implementation. Function GetVariantFromNotStringJSON( is very "optimistically" in the cases like below

  if (JSON=nil) or
     ((PInteger(JSON)^=NULL_LOW) and (JSON[4] in EndOfJSONValueField)) then

gets access to JSON bytes after JSON[1]

Since this optimization technique is very widely used in mORMot (FALSE_LOW, NULL_LOW etc) IMHO the only possible
way to fix is to allocate a TSynTempBuffer 5 bytes longer than the source string length and fill last 5 bytes with 0

@ab - need your help

Last edited by mpv (2018-12-23 13:53:26)

Offline

#2 2018-12-27 11:04:20

mpv
Member
From: Ukraine
Registered: 2012-03-24
Posts: 1,534
Website

Re: Potential AV in _JsonFast

I create a pull request #168 with a small program to reproduce a problem I was writing about.

This pull request fix only _JsonFast() for cases when TSynTempBuffer buffer if used. More elegant solution is required for other cases (TDocVariantData.InitJSONFromFile for example)

Last edited by mpv (2018-12-27 11:08:11)

Offline

#3 2018-12-27 14:04:56

ab
Administrator
From: France
Registered: 2010-06-21
Posts: 14,182
Website

Re: Potential AV in _JsonFast

Please check https://synopse.info/fossil/info/3e501bc977

I hope this fix is wider than your pull request.

Offline

#4 2018-12-27 15:49:42

mpv
Member
From: Ukraine
Registered: 2012-03-24
Posts: 1,534
Website

Re: Potential AV in _JsonFast

Perfect! And with [ac89bf90] even valgrind is happy. Thank you very much

Offline

#5 2019-01-09 15:29:00

chapa
Member
Registered: 2012-04-30
Posts: 117

Re: Potential AV in _JsonFast

yeah, past three years I experience random AVs as described. One on million, but still random AVs, was unable to track them.

mvp, does it resolves this old timer in your opinion:
https://synopse.info/forum/viewtopic.php?id=3283

Just compiled the new sources, and cross fingers for next 24 hours smile

Offline

#6 2019-01-09 15:33:39

ab
Administrator
From: France
Registered: 2010-06-21
Posts: 14,182
Website

Re: Potential AV in _JsonFast

Current trunk seemed more stable than previous.

I had sometimes random A/V during regression tests under Linux / WSL and not any more.

If it continue to occur, try to disable SSE4.2 StrLen and like in SynCommons.

Offline

#7 2019-01-09 15:42:54

mpv
Member
From: Ukraine
Registered: 2012-03-24
Posts: 1,534
Website

Re: Potential AV in _JsonFast

In my case AV occurs when I use JsonFast and numeric field is in the end of JSON {... "userID":1}, very similar to a case from your code [{.. "Seen": 1}]. Since AB fix it not only for JsonFast but also for TTextWriter I hope it solve your problem smile

Offline

#8 2019-01-09 15:59:21

mpv
Member
From: Ukraine
Registered: 2012-03-24
Posts: 1,534
Website

Re: Potential AV in _JsonFast

ab wrote:

Current trunk seemed more stable than previous.

I had sometimes random A/V during regression tests under Linux / WSL and not any more.

If it continue to occur, try to disable SSE4.2 StrLen and like in SynCommons.

I think on Windows AV did not occur because FastMM reuse a previously allocated blocks. Isn't it?

Offline

#9 2019-01-21 09:14:05

chapa
Member
Registered: 2012-04-30
Posts: 117

Re: Potential AV in _JsonFast

@ab I can confirm I get no more random AVs as described!

Thanks once again for the patch.
And to @mvp for finding it, I was unable to and got used to live with it smile

My build is XE3 CE x64 on various Windows.

Offline

#10 2019-01-21 09:22:09

ab
Administrator
From: France
Registered: 2010-06-21
Posts: 14,182
Website

Re: Potential AV in _JsonFast

big_smile

Offline

Board footer

Powered by FluxBB